Repository navigation
Fix in-place URL decoding of interned userinfo in the http and ftp wrappers - #24341
alepacheco wants to merge 1 commit into
Conversation
|
|
||
| #define PHP_FTP_CNTRL_CHK(val, val_len, err_msg) { \ | ||
| unsigned char *s = (unsigned char *) val, *e = (unsigned char *) s + val_len; \ | ||
| #define PHP_FTP_CNTRL_CHK(val, err_msg) { \ |
There was a problem hiding this comment.
It would be nice to convert this macro to a function, just like in http_fopen_wrapper.c
There was a problem hiding this comment.
Done in 6d0afd5: the macro is gone. php_ftp_has_cntrl() does the check, and the two callers log the error, free the decoded string and jump to connect_errexit themselves, as the http wrapper does with php_stream_http_url_component_has_cntrl().
| } | ||
| } | ||
| if ($n < 1) { | ||
| fwrite($conn, "HTTP/1.1 302 Found\r\nLocation: http://+@$addr/b\r\nContent-Length: 0\r\n\r\n"); |
There was a problem hiding this comment.
as far as I can see, the password could be tested similarly, too.
There was a problem hiding this comment.
Good point, added: the test now follows a second redirect whose userinfo is a one-character password (http://a:+@host/d) and checks the Authorization header and the + string after it as well. With only the password decode put back in place, it fails on that second check.
…appers The http and ftp stream wrappers decoded the user and password (and, when checking a redirect, the path) of the parsed URL in place. The components of a php_uri may be interned strings (the WHATWG parser returns one-character usernames and passwords as interned single-char strings, the RFC 3986 parser returns the path "/" as one), so decoding a redirect target such as http://+@host/ rewrote the shared "+" string for the whole process. Decode the components into new strings instead, and build the relative redirect path without writing into the current path.
08f244a to
6d0afd5
Compare
The http and ftp stream wrappers url-decode the user and password of the parsed URL in place (
php_url_decode(ZSTR_VAL(resource->user), ...)), and the http wrapper does the same for user, password and path when it checks a redirect target for control characters.Since the wrappers take a
php_urifromuri_parser_class, those components can be interned strings: the WHATWG parser returns one-character usernames and passwords throughZVAL_STRINGL_FAST(the interned single-char strings), and the RFC 3986 parser returns the path/withZVAL_CHAR. Withuri_parser_class => Uri\WhatWg\Url, a redirect tohttp://+@host/therefore rewrites the shared"+"string to" "for the rest of the process (with opcache, in shared memory).This changes the wrappers to never write into
php_urifields:http_fopen_wrapper.c: the Basic auth header and the redirect control-character check decode into new strings; the relative redirect path is built from the directory part of the current path instead of truncatingresource->pathin place (same resulting URL as before).ftp_fopen_wrapper.c:USER/PASSare sent from decoded copies.The new test follows a redirect with a one-character username under the WHATWG parser and checks both the
Authorizationheader sent and thatchr(43)is still"+"afterwards.Targeting PHP-8.5, the first branch with
uri_parser_class.