Conversation
…cheable_stream_path() memcmp() will read inside the padding bytes of the zend_string, which are not initialized. This is not exploitable because they're 8 padding bytes and the length of "file://" and "phar://" is 7 so the mismatch happens at the length-of-the-string index; returning non-0 anyway.
| return memcmp(filename, "file://", sizeof("file://") - 1) == 0 || | ||
| memcmp(filename, "phar://", sizeof("phar://") - 1) == 0; | ||
| return zend_string_starts_with_literal(filename, "file://") || | ||
| zend_string_starts_with_literal(filename, "phar://"); |
There was a problem hiding this comment.
Not a new issue, but should this be zend_string_starts_with_literal_ci()?
There was a problem hiding this comment.
Nice catch. Dunno which URL standard we follow, but from RFC3986:
Although schemes are case-
insensitive, the canonical form is lowercase and documents that
specify schemes must do so with lowercase letters. An implementation
should accept uppercase letters as equivalent to lowercase in scheme
names (e.g., allow "HTTP" as well as "http") for the sake of
robustness but should only produce lowercase scheme names for
consistency.
So yes, in principle this should be accepted.
Most code in php-src seems to check for this case-insensitively, but e.g. ext/openssl does not.
Should be brought up in a separate issue and uniformized (on consensus).
2 participants
memcmp() will read inside the padding bytes of the zend_string, which are not initialized. This is not exploitable because they're 8 padding bytes and the length of "file://" and "phar://" is 7 so the mismatch happens at the length-of-the-string index; returning non-0 anyway.