Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Internal exceptions may misinterpret the null byte \0 #10810

Closed
b-viguier opened this issue Mar 8, 2023 · 6 comments
Closed

Internal exceptions may misinterpret the null byte \0 #10810

b-viguier opened this issue Mar 8, 2023 · 6 comments

Comments

@b-viguier
Copy link
Contributor

b-viguier commented Mar 8, 2023

Description

The following code:

<?php

throw new \Exception("Hello\0World");

Resulted in this output:

Fatal error: Uncaught Exception: Hello in /tmp/preview:3
Stack trace:
#0 {main}
  thrown in /tmp/preview on line 3

But I expected this output instead:

Fatal error: Uncaught Exception: HelloWorld in /tmp/preview:3
Stack trace:
#0 {main}
  thrown in /tmp/preview on line 3

or

Fatal error: Uncaught Exception: Hello\x00World in /tmp/preview:3
Stack trace:
#0 {main}
  thrown in /tmp/preview on line 3

Notes

Thanks to this strange behavior, I discovered that Zend strings are perfectly able to handle the \0 character.
But to call the function zend_throw_exception(zend_class_entry *exception_ce, const char *message, zend_long code), some callers may convert a zend string to a raw C string, losing the actual size. Then, the newly created zend string will stop at the first \0, loosing the end of the string.

I didn't found any risk with this bug, since sensitive functions seem to look explicitly for this kind of abuse. Just to notice that some error messages may be truncated if they contain this null byte.

Here are some other examples:

<?php

call_user_func("Hello\0World");

$str = "Hello\0World";
new $str;

assert(false, "Hello\0World");

💡 It may be relevant to prefer the function zend_throw_exception_zstr when possible, to prevent to lose the actual length of the zend string.

Thanks for your amazing work 🙂 👍

PHP Version

PHP 8.0, 8.1, 8.2

Operating System

No response

@TimWolla
Copy link
Member

TimWolla commented Mar 8, 2023

As I've noted on phpc.social, the issue is with __toString(). The $message property holds the correct value:

https://3v4l.org/Kt5QS

<?php

$e = new \Exception("Hello\0World");

var_dump(strlen($e->getMessage()));
var_dump($e->getMessage() === "Hello\0World");
var_dump($e->getMessage());

@b-viguier
Copy link
Contributor Author

@TimWolla you're right, I didn't spot this issue with __toString, but I'm not sure it's the exact same problem, because zend_throw_exception doesn't call __toString 🤔

But it's similar, when an exception is converted to string, the message is transmitted as a char* in zend_strpprintf, losing the length stored in the zend string 😕

ZSTR_VAL(Z_OBJCE_P(exception)->name), ZSTR_VAL(message), ZSTR_VAL(file), line,

@mvorisek
Copy link
Contributor

mvorisek commented Mar 9, 2023

userland sprintf can handle zend_string with null byte - https://3v4l.org/dGqpR, can zend_string be used with zend_strpprintf directly?

@iluuu1994
Copy link
Member

iluuu1994 commented Mar 13, 2023

This is actually relatively annoying to fix. zend_strpprintf provides only one way to not stop on \0 characters, namely %Z. However, %Z is not a standard printf argument (obviously) and thus compilers will complain about mismatched placeholder/arg counts. Changing behavior for width (%*s) or precision (%.*s) are both dangerously breaking, potentially leading to a buffer overflow. Using php_formatted_print isn't impossible. However, the code isn't used anywhere else internally, and it operates on zvals. The same problem can also occur in most logs that use zend_string, so a specific fix for just this case doesn't seem sensible.

@iluuu1994
Copy link
Member

I missed that there's already an _unchecked variant for these cases. #10873

@vlakoff
Copy link
Contributor

vlakoff commented Jul 12, 2024

PHP supports "\0" in double-quoted strings, not because it is handled as a special character, but because it is an octal representation.

"\0" is the octal equivalent of the hexadecimal "\x00". (nonsignificant zeroes can be omitted, so \x0 is also valid, but I wouldn't recommend it, as we represent bytes, which consist of 2 hexadecimal digits)

I would not recommend using the octal "\0", as it may be confusing.
For example, "\042" results to the same as "\42" or "\x22", not to the "\0" . "42" that some people may have been expecting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants
@TimWolla @vlakoff @iluuu1994 @mvorisek @b-viguier @nielsdos and others