diff --git a/ext/ftp/tests/server.inc b/ext/ftp/tests/server.inc index 3fc167148a0e..0e600baddd94 100644 --- a/ext/ftp/tests/server.inc +++ b/ext/ftp/tests/server.inc @@ -431,7 +431,7 @@ if ($pid) { $host = "127.0.0.1"; $i=0; - if (empty($bug73457)) { + if (empty($bug73457) && empty($pasv_address_at_line_end)) { if (!empty($ssl)) { $soc = stream_socket_server("tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND|STREAM_SERVER_LISTEN, $context); } else { @@ -450,9 +450,14 @@ if ($pid) { $p2 = $pasv_port % ((int) 1 << 8); $p1 = ($pasv_port-$p2)/((int) 1 << 8); - fputs($s, "227 Entering Passive Mode. (127,0,0,1,{$p1},{$p2})\r\n"); + if (empty($pasv_address_at_line_end)) { + fputs($s, "227 Entering Passive Mode. (127,0,0,1,{$p1},{$p2})\r\n"); + } else { + /* The address starts 14 bytes before the end of the 512 bytes the client reads the line into */ + fputs($s, "227 " . str_repeat('.', 494) . "127,0,0,1,1,1)\r\n"); + } - if (empty($bug73457)) { + if (empty($bug73457) && empty($pasv_address_at_line_end)) { $pasvs = stream_socket_accept($soc,10); } else { $pasvs = null; @@ -591,6 +596,11 @@ if ($pid) { fclose($fs); }elseif (preg_match('/^SIZE \/bug73457/', $buf)) { fputs($s, "213 10\r\n"); + }elseif (preg_match('/^SIZE \/mdtmnodigits/', $buf)) { + fputs($s, "213 10\r\n"); + }elseif (preg_match('/^MDTM \/mdtmnodigits/', $buf)) { + // No digit in the whole 512 byte buffer the client reads the reply into + fputs($s, "213 " . str_repeat('x', 600) . "\r\n"); }elseif (preg_match("/^SITE/", $buf)) { fputs($s, "500 Syntax error, command unrecognized.\r\n"); }else { diff --git a/ext/mysqli/tests/fake_server.inc b/ext/mysqli/tests/fake_server.inc index 2a4d17f946e8..26bcf7d346ee 100644 --- a/ext/mysqli/tests/fake_server.inc +++ b/ext/mysqli/tests/fake_server.inc @@ -805,6 +805,30 @@ function my_mysqli_test_stmt_response_row_short_length( $conn->read(65536); } +function my_mysqli_test_stmt_response_row_no_null_bitmap(my_mysqli_fake_server_conn $conn): void +{ + $rh = $conn->packet_generator->server_stmt_execute_data_response('intval'); + + // The row packet ends right after the status byte, before the NULL bitmap + $row = new my_mysqli_fake_packet(); + $row->packet_length = '010000'; + $row->packet_number = '05'; + $row->packet_type = '00'; + $rh[4] = $row; + + $conn->send_server_greetings(); + $conn->read(); + $conn->send_server_ok(); + $conn->read(); + $conn->send_server_stmt_prepare_data_response('intval'); + $conn->read(); + $conn->send( + $conn->packets_to_bytes($rh), + "Malicious Stmt Response for data intval [no NULL bitmap]" + ); + $conn->read(65536); +} + function my_mysqli_test_stmt_response_row_short_length_date(my_mysqli_fake_server_conn $conn): void { my_mysqli_test_stmt_response_row_short_length($conn, 'datval'); diff --git a/ext/mysqli/tests/mysqlnd_stmt_row_no_null_bitmap.phpt b/ext/mysqli/tests/mysqlnd_stmt_row_no_null_bitmap.phpt new file mode 100644 index 000000000000..21ed51887f1c --- /dev/null +++ b/ext/mysqli/tests/mysqlnd_stmt_row_no_null_bitmap.phpt @@ -0,0 +1,45 @@ +--TEST-- +mysqlnd rejects a binary protocol row packet too short for its NULL bitmap +--EXTENSIONS-- +mysqli +--FILE-- +wait(); + +$conn = new mysqli($servername, $username, $password, "", $process->getPort()); + +echo "[*] Prepare and execute on the fake server...\n"; +$stmt = $conn->prepare("SELECT strval, intval FROM data"); +$stmt->execute(); +$result = $stmt->get_result(); +while ($row = $result->fetch_row()) { + var_dump($row); +} +$stmt->close(); +$conn->close(); + +$process->terminate(); + +print "done!"; +?> +--EXPECTF-- +[*] Server started on 127.0.0.1:%d +[*] Connection established +[*] Sending - Server Greeting: 580000000a352e352e352d31302e352e31382d4d6172696144420003000000473e3f6047257c6700fef7080200ff81150000000000000f0000006c6b55463f49335f686c6431006d7973716c5f6e61746976655f70617373776f7264 +[*] Received: 6900000185a21a00000000c0080000000000000000000000000000000000000000000000726f6f7400006d7973716c5f6e61746976655f70617373776f7264002c0c5f636c69656e745f6e616d65076d7973716c6e640c5f7365727665725f686f7374093132372e302e302e31 +[*] Sending - Server OK: 0700000200000002000000 +[*] Prepare and execute on the fake server... +[*] Received: 200000001653454c4543542073747276616c2c20696e7476616c2046524f4d2064617461 +[*] Sending - Stmt prepare data intval: 0c0000010001000000020000000000003200000203646566087068705f74657374046461746104646174610673747276616c0673747276616c0ce000c8000000fd01100000003200000303646566087068705f746573740464617461046461746106696e7476616c06696e7476616c0c3f000b00000003011000000005000004fe00000200 +[*] Received: 0a00000017010000000001000000 +[*] Sending - Malicious Stmt Response for data intval [no NULL bitmap]: 01000001023200000203646566087068705f74657374046461746104646174610673747276616c0673747276616c0ce000c8000000fd01100000003200000303646566087068705f746573740464617461046461746106696e7476616c06696e7476616c0c3f000b00000003011000000005000004fe00002200010000050005000006fe00002200 + +Warning: mysqli_result::fetch_row(): Malformed server packet. No packet space left for the NULL bitmap in %s on line %d +done! diff --git a/ext/mysqlnd/mysqlnd_wireprotocol.c b/ext/mysqlnd/mysqlnd_wireprotocol.c index 65b0bceae591..46b308742df7 100644 --- a/ext/mysqlnd/mysqlnd_wireprotocol.c +++ b/ext/mysqlnd/mysqlnd_wireprotocol.c @@ -1546,6 +1546,12 @@ php_mysqlnd_rowp_read_binary_protocol(MYSQLND_ROW_BUFFER * row_buffer, zval * fi end_field = (start_field = fields) + field_count; + /* The status byte and the NULL bitmap must fit in the packet */ + if (UNEXPECTED(rbs < 1 + (field_count + 9) / 8)) { + php_error_docref(NULL, E_WARNING, "Malformed server packet. No packet space left for the NULL bitmap"); + DBG_RETURN(FAIL); + } + /* skip the first byte, not EODATA_MARKER -> 0x0, status */ p++; null_ptr= p; diff --git a/ext/soap/php_http.c b/ext/soap/php_http.c index d64a110cb1c8..a4f597c16933 100644 --- a/ext/soap/php_http.c +++ b/ext/soap/php_http.c @@ -347,6 +347,44 @@ static bool in_domain(const zend_string *host, const zend_string *domain) } } +/* Whether a cookie Domain attribute received from host may be accepted (RFC 6265 5.1.3) */ +static bool cookie_domain_matches(const zend_string *host, const char *domain, size_t domain_len) +{ + if (domain_len > 0 && domain[0] == '.') { + domain++; + domain_len--; + } + + if (domain_len == 0 || domain_len > ZSTR_LEN(host)) { + return false; + } + + if (strncasecmp(ZSTR_VAL(host) + ZSTR_LEN(host) - domain_len, domain, domain_len) != 0) { + return false; + } + + return domain_len == ZSTR_LEN(host) || ZSTR_VAL(host)[ZSTR_LEN(host) - domain_len - 1] == '.'; +} + +/* The port of from must already be resolved, the port of to may still be 0 */ +static bool same_origin(const php_uri *from, const php_uri *to) +{ + if (from->scheme == NULL || to->scheme == NULL || from->host == NULL || to->host == NULL) { + return false; + } + + if (!zend_string_equals_ci(from->scheme, to->scheme) || !zend_string_equals_ci(from->host, to->host)) { + return false; + } + + zend_long to_port = to->port; + if (to_port == 0) { + to_port = zend_string_equals_literal_ci(to->scheme, "https") ? 443 : 80; + } + + return from->port == to_port; +} + bool make_http_soap_request( zval *this_ptr, zend_string *buf, zend_string *location, char *soapaction, int soap_version, zend_string *uri_parser_class, zval *return_value @@ -376,6 +414,8 @@ bool make_http_soap_request( bool has_authorization = false; bool has_proxy_authorization = false; bool has_cookies = false; + /* Set once a redirect left the origin, credentials must not follow it */ + bool strip_auth = false; if (this_ptr == NULL || Z_TYPE_P(this_ptr) != IS_OBJECT) { return false; @@ -687,7 +727,7 @@ bool make_http_soap_request( /* HTTP Authentication */ login = Z_CLIENT_LOGIN_P(this_ptr); - if (Z_TYPE_P(login) == IS_STRING) { + if (Z_TYPE_P(login) == IS_STRING && !strip_auth) { zval *digest = Z_CLIENT_DIGEST_P(this_ptr); has_authorization = true; @@ -790,16 +830,16 @@ bool make_http_soap_request( make_digest(response, hash); smart_str_append_const(&soap_headers, "Authorization: Digest username=\""); - smart_str_append(&soap_headers, Z_STR_P(login)); + soap_smart_str_append_header_value(&soap_headers, Z_STR_P(login), "Authorization"); if ((tmp = zend_hash_str_find(Z_ARRVAL_P(digest), "realm", sizeof("realm")-1)) != NULL && Z_TYPE_P(tmp) == IS_STRING) { smart_str_append_const(&soap_headers, "\", realm=\""); - smart_str_append(&soap_headers, Z_STR_P(tmp)); + soap_smart_str_append_header_value(&soap_headers, Z_STR_P(tmp), "Authorization"); } if ((tmp = zend_hash_str_find(Z_ARRVAL_P(digest), "nonce", sizeof("nonce")-1)) != NULL && Z_TYPE_P(tmp) == IS_STRING) { smart_str_append_const(&soap_headers, "\", nonce=\""); - smart_str_append(&soap_headers, Z_STR_P(tmp)); + soap_smart_str_append_header_value(&soap_headers, Z_STR_P(tmp), "Authorization"); } smart_str_append_const(&soap_headers, "\", uri=\""); if (uri->path) { @@ -829,12 +869,12 @@ bool make_http_soap_request( if ((tmp = zend_hash_str_find(Z_ARRVAL_P(digest), "opaque", sizeof("opaque")-1)) != NULL && Z_TYPE_P(tmp) == IS_STRING) { smart_str_append_const(&soap_headers, "\", opaque=\""); - smart_str_append(&soap_headers, Z_STR_P(tmp)); + soap_smart_str_append_header_value(&soap_headers, Z_STR_P(tmp), "Authorization"); } if ((tmp = zend_hash_str_find(Z_ARRVAL_P(digest), "algorithm", sizeof("algorithm")-1)) != NULL && Z_TYPE_P(tmp) == IS_STRING) { smart_str_append_const(&soap_headers, "\", algorithm=\""); - smart_str_append(&soap_headers, Z_STR_P(tmp)); + soap_smart_str_append_header_value(&soap_headers, Z_STR_P(tmp), "Authorization"); } smart_str_append_const(&soap_headers, "\"\r\n"); } else { @@ -899,7 +939,9 @@ bool make_http_soap_request( smart_str_append_const(&soap_headers, "\r\n"); } - http_context_headers(context, has_authorization, has_proxy_authorization, has_cookies, &soap_headers); + /* Passing true for the credential headers makes it skip them */ + http_context_headers(context, has_authorization || strip_auth, has_proxy_authorization, + has_cookies || strip_auth, &soap_headers); smart_str_append_const(&soap_headers, "\r\n"); smart_str_0(&soap_headers); @@ -1026,6 +1068,7 @@ bool make_http_soap_request( if (eqpos != NULL && (sempos == NULL || sempos > eqpos)) { zval zcookie; size_t cookie_value_len; + bool foreign_domain = false; if (sempos != NULL) { cookie_value_len = sempos-(eqpos+1); @@ -1048,7 +1091,13 @@ bool make_http_soap_request( add_index_stringl(&zcookie, 1, eqpos, sempos?(size_t)(sempos-eqpos):strlen(eqpos)); } else if (strstr(options,"domain=") == options) { eqpos = options + sizeof("domain=")-1; - add_index_stringl(&zcookie, 2, eqpos, sempos?(size_t)(sempos-eqpos):strlen(eqpos)); + size_t domain_len = sempos ? (size_t)(sempos-eqpos) : strlen(eqpos); + /* A server must not set cookies for other hosts */ + if (!cookie_domain_matches(uri->host, eqpos, domain_len)) { + foreign_domain = true; + break; + } + add_index_stringl(&zcookie, 2, eqpos, domain_len); } else if (strstr(options,"secure") == options) { add_index_bool(&zcookie, 3, 1); } @@ -1059,19 +1108,23 @@ bool make_http_soap_request( } } } - if (!zend_hash_index_exists(Z_ARRVAL(zcookie), 1)) { - char *t = uri->path?ZSTR_VAL(uri->path):"/"; - char *c = strrchr(t, '/'); - if (c) { - add_index_stringl(&zcookie, 1, t, c-t); + if (foreign_domain) { + zval_ptr_dtor(&zcookie); + } else { + if (!zend_hash_index_exists(Z_ARRVAL(zcookie), 1)) { + char *t = uri->path?ZSTR_VAL(uri->path):"/"; + char *c = strrchr(t, '/'); + if (c) { + add_index_stringl(&zcookie, 1, t, c-t); + } + } + if (!zend_hash_index_exists(Z_ARRVAL(zcookie), 2)) { + add_index_str(&zcookie, 2, uri->host); + GC_ADDREF(uri->host); } - } - if (!zend_hash_index_exists(Z_ARRVAL(zcookie), 2)) { - add_index_str(&zcookie, 2, uri->host); - GC_ADDREF(uri->host); - } - zend_symtable_update(Z_ARRVAL_P(cookies), name, &zcookie); + zend_symtable_update(Z_ARRVAL_P(cookies), name, &zcookie); + } zend_string_release_ex(name, false); } @@ -1191,6 +1244,9 @@ bool make_http_soap_request( } } } + if (!same_origin(uri, new_uri)) { + strip_auth = true; + } uri = new_uri; if (--redirect_max < 1) { @@ -1210,7 +1266,7 @@ bool make_http_soap_request( zval *password = Z_CLIENT_PASSWORD_P(this_ptr); char *auth = get_http_header_value(http_headers, "WWW-Authenticate:"); if (auth && strstr(auth, "Digest") == auth && Z_TYPE_P(digest) != IS_ARRAY - && Z_TYPE_P(login) == IS_STRING && Z_TYPE_P(password) == IS_STRING) { + && Z_TYPE_P(login) == IS_STRING && Z_TYPE_P(password) == IS_STRING && !strip_auth) { char *s; zval digest; diff --git a/ext/soap/tests/bugs/cookie_parse_options_offset.phpt b/ext/soap/tests/bugs/cookie_parse_options_offset.phpt index 988af9d31959..001f9a984670 100644 --- a/ext/soap/tests/bugs/cookie_parse_options_offset.phpt +++ b/ext/soap/tests/bugs/cookie_parse_options_offset.phpt @@ -23,7 +23,7 @@ if (php_ini_loaded_file()) { // falsely matching it as a path attribute. $code = <<<'PHP' header("Content-Type: text/xml"); -header("Set-Cookie: sessionkey=path=/evil;domain=good.com"); +header("Set-Cookie: sessionkey=path=/evil;domain=localhost"); echo << @@ -58,4 +58,4 @@ echo "domain: " . $cookies['sessionkey'][2] . "\n"; --EXPECT-- value: path=/evil path: /test -domain: good.com +domain: localhost diff --git a/ext/soap/tests/bugs/redirect_strip_credentials.phpt b/ext/soap/tests/bugs/redirect_strip_credentials.phpt new file mode 100644 index 000000000000..89e554c71082 --- /dev/null +++ b/ext/soap/tests/bugs/redirect_strip_credentials.phpt @@ -0,0 +1,90 @@ +--TEST-- +SOAP client does not send credentials to another origin after a redirect +--EXTENSIONS-- +soap +--SKIPIF-- + +--FILE-- +' + . '' + . 'ok' + . ''; +$ok = "data://text/plain,HTTP/1.1 200 OK\r\nContent-Type: text/xml\r\nContent-Length: " . strlen($response) . "\r\n\r\n" . $response; + +$ctx = stream_context_create(['http' => [ + 'header' => "Authorization: Bearer SECRET\r\nCookie: sid=abc\r\nX-Custom: keep-me", +]]); + +function call(string $location, $ctx): void { + $client = new SoapClient(null, [ + 'location' => $location, + 'uri' => 'test-uri', + 'login' => 'user', + 'password' => 'secret', + 'stream_context' => $ctx, + 'keep_alive' => false, + ]); + var_dump($client->__soapCall('foo', [])); +} + +/* B is on another port than A, so the hop from A to B is cross-origin; B then + * redirects to itself, credentials must stay withheld for that hop too */ +['pid' => $pidB, 'uri' => $uriB] = http_server([ + "data://text/plain,HTTP/1.1 302 Found\r\nLocation: /second\r\nContent-Length: 0\r\n\r\n", + $ok, +], $captureB); +['pid' => $pidA, 'uri' => $uriA] = http_server([ + "data://text/plain,HTTP/1.1 302 Found\r\nLocation: $uriB/first\r\nContent-Length: 0\r\n\r\n", +], $captureA); + +call("$uriA/src", $ctx); + +http_server_kill($pidA); +http_server_kill($pidB); + +rewind($captureA); +rewind($captureB); +report('--- origin A (1 request) ---', stream_get_contents($captureA)); +report('--- origin B (2 requests) ---', stream_get_contents($captureB)); + +/* Same origin throughout: credentials are sent on both hops */ +['pid' => $pidC, 'uri' => $uriC] = http_server([ + "data://text/plain,HTTP/1.1 302 Found\r\nLocation: /next\r\nContent-Length: 0\r\n\r\n", + $ok, +], $captureC); + +call("$uriC/first", $ctx); + +http_server_kill($pidC); + +rewind($captureC); +report('--- origin C (2 requests) ---', stream_get_contents($captureC)); +?> +--EXPECT-- +string(2) "ok" +--- origin A (1 request) --- + Authorization: 1 + Cookie: 1 + X-Custom: 1 +--- origin B (2 requests) --- + Authorization: 0 + Cookie: 0 + X-Custom: 2 +string(2) "ok" +--- origin C (2 requests) --- + Authorization: 2 + Cookie: 2 + X-Custom: 2 diff --git a/ext/soap/tests/cookie_foreign_domain.phpt b/ext/soap/tests/cookie_foreign_domain.phpt new file mode 100644 index 000000000000..2ed374897aca --- /dev/null +++ b/ext/soap/tests/cookie_foreign_domain.phpt @@ -0,0 +1,49 @@ +--TEST-- +SoapClient ignores cookies set by a server for a domain that does not match its host +--EXTENSIONS-- +soap +--SKIPIF-- + +--FILE-- +' + . '' + . 'ok' + . ''; + +['pid' => $pid, 'uri' => $uri] = http_server(function ($server) use ($response) { + $host = parse_url('http://' . stream_socket_get_name($server, false), PHP_URL_HOST); + yield "data://text/plain,HTTP/1.1 200 OK\r\n" + . "Set-Cookie: foreign=1; domain=evil.example\r\n" + . "Set-Cookie: suffix=2; domain=.ple\r\n" + . "Set-Cookie: own=3; domain=$host\r\n" + . "Set-Cookie: hostonly=4; path=/\r\n" + . "Content-Type: text/xml\r\nContent-Length: " . strlen($response) . "\r\n\r\n" . $response; +}); + +$client = new SoapClient(null, [ + 'location' => "$uri/", + 'uri' => 'test-uri', + 'keep_alive' => false, +]); +var_dump($client->__soapCall('foo', [])); + +http_server_kill($pid); + +$cookies = array_keys($client->__getCookies()); +sort($cookies); +var_dump($cookies); +?> +--EXPECT-- +string(2) "ok" +array(2) { + [0]=> + string(8) "hostonly" + [1]=> + string(3) "own" +} diff --git a/ext/soap/tests/digest_auth_header_injection.phpt b/ext/soap/tests/digest_auth_header_injection.phpt new file mode 100644 index 000000000000..0db49958fa2c --- /dev/null +++ b/ext/soap/tests/digest_auth_header_injection.phpt @@ -0,0 +1,49 @@ +--TEST-- +SoapClient must truncate Digest authentication values that contain newline characters +--EXTENSIONS-- +soap +--SKIPIF-- + +--FILE-- +' + . '' + . 'ok' + . ''; + +['pid' => $pid, 'uri' => $uri] = http_server([ + "data://text/plain,HTTP/1.1 401 Unauthorized\r\nWWW-Authenticate: Digest realm=\"r\rX-Realm: yes\", nonce=\"n\", qop=\"auth\"\r\nContent-Length: 0\r\n\r\n", + "data://text/plain,HTTP/1.1 200 OK\r\nContent-Type: text/xml\r\nContent-Length: " . strlen($response) . "\r\n\r\n" . $response, +], $capture); + +$client = new SoapClient(null, [ + 'location' => "$uri/", + 'uri' => 'test-uri', + 'login' => "user\r\nX-Injected: yes", + 'password' => 'secret', + 'authentication' => SOAP_AUTHENTICATION_DIGEST, + 'keep_alive' => false, +]); +var_dump($client->__soapCall('foo', [])); + +http_server_kill($pid); + +rewind($capture); +$requests = stream_get_contents($capture); +var_dump(preg_match_all('/^Authorization: Digest username="user", realm="r", nonce="n"/m', $requests)); +var_dump(preg_match('/^X-Injected:/m', $requests)); +var_dump(str_contains($requests, "\rX-Realm")); +?> +--EXPECTF-- +Warning: SoapClient::__doRequest(): Header Authorization value contains newline characters and has been truncated in %s on line %d + +Warning: SoapClient::__doRequest(): Header Authorization value contains newline characters and has been truncated in %s on line %d +string(2) "ok" +int(1) +int(0) +bool(false) diff --git a/ext/standard/ftp_fopen_wrapper.c b/ext/standard/ftp_fopen_wrapper.c index 97879a5c7000..146c92129f3b 100644 --- a/ext/standard/ftp_fopen_wrapper.c +++ b/ext/standard/ftp_fopen_wrapper.c @@ -364,8 +364,8 @@ static unsigned short php_fopen_do_pasv(php_stream *stream, char *ip, size_t ip_ tpath++; } tpath[-1] = '\0'; - memcpy(ip, hoststart, ip_size); - ip[ip_size-1] = '\0'; + /* hoststart may be closer than ip_size to the end of tmp_line */ + strlcpy(ip, hoststart, ip_size); hoststart = ip; /* pull out the MSB of the port */ @@ -847,11 +847,11 @@ static int php_stream_ftp_url_stat(php_stream_wrapper *wrapper, const char *url, struct tm tm, tmbuf, *gmt; time_t stamp; - while ((size_t)(p - tmp_line) < sizeof(tmp_line) && !isdigit((unsigned char)*p)) { + while (*p != '\0' && !isdigit((unsigned char)*p)) { p++; } - if ((size_t)(p - tmp_line) > sizeof(tmp_line)) { + if (*p == '\0') { goto mdtm_error; } diff --git a/ext/standard/tests/streams/ftp_pasv_address_at_line_end.phpt b/ext/standard/tests/streams/ftp_pasv_address_at_line_end.phpt new file mode 100644 index 000000000000..01bdb509a99f --- /dev/null +++ b/ext/standard/tests/streams/ftp_pasv_address_at_line_end.phpt @@ -0,0 +1,22 @@ +--TEST-- +FTP wrapper must not read past the response buffer when the PASV address ends the line +--SKIPIF-- + +--INI-- +default_socket_timeout=1 +--FILE-- + +--EXPECTF-- +Warning: file_get_contents(): Failed to open stream: Failed to set up data channel: %s in %s on line %d +bool(false) diff --git a/ext/standard/tests/streams/ftp_stat_mdtm_no_digits.phpt b/ext/standard/tests/streams/ftp_stat_mdtm_no_digits.phpt new file mode 100644 index 000000000000..38d3e41b7b1c --- /dev/null +++ b/ext/standard/tests/streams/ftp_stat_mdtm_no_digits.phpt @@ -0,0 +1,20 @@ +--TEST-- +stat() on ftp:// must not read past the response buffer for an MDTM reply without digits +--SKIPIF-- + +--FILE-- + +--EXPECT-- +int(10) +int(-1) diff --git a/sapi/fpm/fpm/fpm_log.c b/sapi/fpm/fpm/fpm_log.c index 87fe4c1d05d7..2fa510cbf8ec 100644 --- a/sapi/fpm/fpm/fpm_log.c +++ b/sapi/fpm/fpm/fpm_log.c @@ -26,6 +26,30 @@ static struct zlog_stream fpm_log_stream; static int fpm_access_log_suppress(struct fpm_scoreboard_proc_s *proc); +/* Writes a value that comes from the request with the control characters escaped, + * so that it cannot start a new line in the access log */ +static void fpm_log_stream_request_value(struct zlog_stream *stream, const char *value) +{ + const char *run = value, *p; + char escaped[sizeof("\\xff")]; + + for (p = value; *p != '\0'; p++) { + unsigned char c = (unsigned char) *p; + if (c >= 0x20 && c != 0x7f) { + continue; + } + if (p > run) { + zlog_stream_str(stream, run, p - run); + } + snprintf(escaped, sizeof(escaped), "\\x%02x", c); + zlog_stream_str(stream, escaped, sizeof(escaped) - 1); + run = p + 1; + } + if (p > run) { + zlog_stream_str(stream, run, p - run); + } +} + int fpm_log_open(int reopen) /* {{{ */ { struct fpm_worker_pool_s *wp; @@ -235,14 +259,14 @@ int fpm_log_write(char *log_format) /* {{{ */ if (!test) { char *env = fcgi_getenv((fcgi_request*) SG(server_context), format, strlen(format)); - zlog_stream_cstr(stream, env ? env : "-"); + fpm_log_stream_request_value(stream, env ? env : "-"); } format[0] = '\0'; break; case 'f': /* script */ if (!test) { - zlog_stream_cstr(stream, *proc.script_filename ? proc.script_filename : "-"); + fpm_log_stream_request_value(stream, *proc.script_filename ? proc.script_filename : "-"); } break; @@ -254,7 +278,7 @@ int fpm_log_write(char *log_format) /* {{{ */ case 'm': /* method */ if (!test) { - zlog_stream_cstr(stream, *proc.request_method ? proc.request_method : "-"); + fpm_log_stream_request_value(stream, *proc.request_method ? proc.request_method : "-"); } break; @@ -352,7 +376,7 @@ int fpm_log_write(char *log_format) /* {{{ */ case 'q': /* query_string */ if (!test) { - zlog_stream_cstr(stream, proc.query_string); + fpm_log_stream_request_value(stream, proc.query_string); } break; @@ -364,7 +388,7 @@ int fpm_log_write(char *log_format) /* {{{ */ case 'r': /* request URI */ if (!test) { - zlog_stream_cstr(stream, proc.request_uri); + fpm_log_stream_request_value(stream, proc.request_uri); } break; @@ -402,7 +426,7 @@ int fpm_log_write(char *log_format) /* {{{ */ case 'u': /* remote user */ if (!test) { - zlog_stream_cstr(stream, proc.auth_user); + fpm_log_stream_request_value(stream, proc.auth_user); } break; diff --git a/sapi/fpm/fpm/fpm_status.c b/sapi/fpm/fpm/fpm_status.c index 4e92198b757d..533550d4b0dd 100644 --- a/sapi/fpm/fpm/fpm_status.c +++ b/sapi/fpm/fpm/fpm_status.c @@ -122,6 +122,33 @@ int fpm_status_export_to_zval(zval *status) } /* }}} */ +/* Encodes a request value for the output format. The encoded string, if any, + * is stored in tmp and must be freed by the caller. */ +static char *fpm_status_encode_value(char *value, bool encode_html, bool encode_json, zend_string **tmp) +{ + char *encoded; + + *tmp = NULL; + if (encode_html) { + *tmp = php_escape_html_entities_ex((const unsigned char *) value, strlen(value), 1, + ENT_DISALLOWED | ENT_HTML_DOC_XML1 | ENT_COMPAT, NULL, /* double_encode */ 1, /* quiet */ 0); + return ZSTR_VAL(*tmp); + } + + if (encode_json) { + *tmp = php_json_encode_string(value, strlen(value), PHP_JSON_INVALID_UTF8_IGNORE); + encoded = ZSTR_VAL(*tmp); + /* remove quotes around the string */ + if (ZSTR_LEN(*tmp) >= 2) { + encoded[ZSTR_LEN(*tmp) - 1] = '\0'; + ++encoded; + } + return encoded; + } + + return value; +} + int fpm_status_handle_request(void) /* {{{ */ { struct fpm_scoreboard_s *scoreboard_p; @@ -522,8 +549,10 @@ int fpm_status_handle_request(void) /* {{{ */ if (full_syntax) { unsigned int i; int first; - zend_string *tmp_query_string, *tmp_request_uri_string; - char *query_string, *request_uri_string; + zend_string *tmp_request_method_string, *tmp_request_uri_string, *tmp_query_string; + zend_string *tmp_auth_user_string, *tmp_script_filename_string; + char *request_method_string, *request_uri_string, *query_string; + char *auth_user_string, *script_filename_string; struct timeval duration, now; float cpu; @@ -548,51 +577,30 @@ int fpm_status_handle_request(void) /* {{{ */ } } - request_uri_string = NULL; - tmp_request_uri_string = NULL; + /* All of these come from the client */ + request_method_string = request_uri_string = auth_user_string = script_filename_string = "-"; + query_string = NULL; + tmp_request_method_string = tmp_request_uri_string = tmp_query_string = NULL; + tmp_auth_user_string = tmp_script_filename_string = NULL; + if (proc->request_method[0] != '\0') { + request_method_string = fpm_status_encode_value(proc->request_method, + encode_html, encode_json, &tmp_request_method_string); + } if (proc->request_uri[0] != '\0') { - if (encode_html) { - tmp_request_uri_string = php_escape_html_entities_ex( - (const unsigned char *) proc->request_uri, - strlen(proc->request_uri), 1, ENT_DISALLOWED | ENT_HTML_DOC_XML1 | ENT_COMPAT, - NULL, /* double_encode */ 1, /* quiet */ 0); - request_uri_string = ZSTR_VAL(tmp_request_uri_string); - } else if (encode_json) { - tmp_request_uri_string = php_json_encode_string(proc->request_uri, - strlen(proc->request_uri), PHP_JSON_INVALID_UTF8_IGNORE); - request_uri_string = ZSTR_VAL(tmp_request_uri_string); - /* remove quotes around the string */ - if (ZSTR_LEN(tmp_request_uri_string) >= 2) { - request_uri_string[ZSTR_LEN(tmp_request_uri_string) - 1] = '\0'; - ++request_uri_string; - } - } else { - request_uri_string = proc->request_uri; - } + request_uri_string = fpm_status_encode_value(proc->request_uri, + encode_html, encode_json, &tmp_request_uri_string); } - - query_string = NULL; - tmp_query_string = NULL; if (proc->query_string[0] != '\0') { - if (encode_html) { - tmp_query_string = php_escape_html_entities_ex( - (const unsigned char *) proc->query_string, - strlen(proc->query_string), 1, ENT_DISALLOWED | ENT_HTML_DOC_XML1 | ENT_COMPAT, - NULL, /* double_encode */ 1, /* quiet */ 0); - } else if (encode_json) { - tmp_query_string = php_json_encode_string(proc->query_string, - strlen(proc->query_string), PHP_JSON_INVALID_UTF8_IGNORE); - } else { - query_string = proc->query_string; - } - if (tmp_query_string) { - query_string = ZSTR_VAL(tmp_query_string); - /* remove quotes around the string */ - if (encode_json && ZSTR_LEN(tmp_query_string) >= 2) { - query_string[ZSTR_LEN(tmp_query_string) - 1] = '\0'; - ++query_string; - } - } + query_string = fpm_status_encode_value(proc->query_string, + encode_html, encode_json, &tmp_query_string); + } + if (proc->auth_user[0] != '\0') { + auth_user_string = fpm_status_encode_value(proc->auth_user, + encode_html, encode_json, &tmp_auth_user_string); + } + if (proc->script_filename[0] != '\0') { + script_filename_string = fpm_status_encode_value(proc->script_filename, + encode_html, encode_json, &tmp_script_filename_string); } /* prevent NaN */ @@ -615,24 +623,33 @@ int fpm_status_handle_request(void) /* {{{ */ (unsigned long) (now_epoch - proc->start_epoch), proc->requests, (unsigned long) (duration.tv_sec * 1000000UL + duration.tv_usec), - proc->request_method[0] != '\0' ? proc->request_method : "-", - request_uri_string ? request_uri_string : "-", + request_method_string, + request_uri_string, query_string ? "?" : "", query_string ? query_string : "", proc->content_length, - proc->auth_user[0] != '\0' ? proc->auth_user : "-", - proc->script_filename[0] != '\0' ? proc->script_filename : "-", + auth_user_string, + script_filename_string, proc->request_stage == FPM_REQUEST_ACCEPTING ? cpu : 0., proc->request_stage == FPM_REQUEST_ACCEPTING ? proc->memory : 0); PUTS(buffer); efree(buffer); + if (tmp_request_method_string) { + zend_string_free(tmp_request_method_string); + } if (tmp_request_uri_string) { zend_string_free(tmp_request_uri_string); } if (tmp_query_string) { zend_string_free(tmp_query_string); } + if (tmp_auth_user_string) { + zend_string_free(tmp_auth_user_string); + } + if (tmp_script_filename_string) { + zend_string_free(tmp_script_filename_string); + } } if (full_post) { diff --git a/sapi/fpm/tests/log-access-escape-control-chars.phpt b/sapi/fpm/tests/log-access-escape-control-chars.phpt new file mode 100644 index 000000000000..79a7c8394549 --- /dev/null +++ b/sapi/fpm/tests/log-access-escape-control-chars.phpt @@ -0,0 +1,54 @@ +--TEST-- +FPM: access log escapes the control characters of request values +--SKIPIF-- + +--FILE-- +start(['--prefix', $prefix]); +$tester->expectLogStartNotices(); +$tester->request( + query: "a=1\r\nforged", + headers: [ + 'HTTP_AUTHORIZATION' => 'Basic ' . base64_encode("user\n127.0.0.1 - admin:pass"), + 'HTTP_X_TEST' => "x\x1by\x7f", + ], +)->expectBody('OK'); +$tester->expectAccessLog( + "user\\x0a127.0.0.1 - admin 'GET /log-access-escape-control-chars.src.php?a=1\\x0d\\x0aforged' x\\x1by\\x7f 200" +); +$tester->terminate(); +$tester->expectLogTerminatingNotices(); +$tester->close(); +$tester->checkAccessLog(); + +?> +Done +--EXPECT-- +Done +--CLEAN-- + diff --git a/sapi/fpm/tests/status-full-encode-request-values.phpt b/sapi/fpm/tests/status-full-encode-request-values.phpt new file mode 100644 index 000000000000..ce7e0257027f --- /dev/null +++ b/sapi/fpm/tests/status-full-encode-request-values.phpt @@ -0,0 +1,64 @@ +--TEST-- +FPM: status encodes the request method, user and script of the processes +--SKIPIF-- + +--FILE-- +start(); +$tester->expectLogStartNotices(); +foreach (['html' => 'text/html', 'xml' => 'text/xml', 'json' => 'application/json'] as $format => $type) { + $responses = $tester + ->multiRequest([ + ['headers' => [ + 'REQUEST_METHOD' => 'GET', + 'HTTP_AUTHORIZATION' => 'Basic ' . base64_encode('":pass'), + ]], + ['uri' => '/status', 'query' => "full&$format", 'delay' => 100000], + ]); + $body = $responses[1]->getBody($type); + echo $format, ": "; + if ($format === 'json') { + $processes = json_decode($body, true)['processes']; + var_dump(in_array('GET"', array_column($processes, 'user'), true)); + } else { + var_dump(strpos($body, '