Skip to content

GH-22905: avoid truncation on null bytes in reflection exceptions - #22922

Open
DanielEScherzer wants to merge 2 commits into
php:PHP-8.4from
DanielEScherzer:reflection-error-truncation
Open

GH-22905: avoid truncation on null bytes in reflection exceptions#22922
DanielEScherzer wants to merge 2 commits into
php:PHP-8.4from
DanielEScherzer:reflection-error-truncation

Conversation

@DanielEScherzer

Copy link
Copy Markdown
Member

In addition to the fixes within the reflection extension, support for error strings with null bytes was added to zend_throw_error() and zend_throw_exception_ex(). The changes to zend_throw_exception_ex() are a partial backport of #16684.

Fixes #22905

So that when the outputs are fixed the changes can be confirmed in tests
In addition to the fixes within the reflection extension, support for error
strings with null bytes was added to `zend_throw_error()` and
`zend_throw_exception_ex()`. The changes to `zend_throw_exception_ex()` are a
partial backport of php#16684.

@Girgias Girgias left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if for a lot of those methods it wouldn't be better to just reject names with nul bytes? As I don't know if anything other than properties can include such a byte?


?>
--EXPECTF--
Fatal error: Uncaught ReflectionException: Class "foo%0bar" does not exist in %s:%d

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to have a class name with null bytes? If not then just using the Path ZPP specifier would be better


class Demo {}
$r = new ReflectionClass(Demo::class);
$r->getAttributes("foo\0bar", ReflectionAttribute::IS_INSTANCEOF);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess one could have an attribute with a NUL byte via eval?


?>
--EXPECTF--
Fatal error: Uncaught ReflectionException: Method Demo::foo%0bar() does not exist in %s:%d

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to have a method with a null byte? If not using path ZPP might be better.


class Demo {}
$r = new ReflectionClass(Demo::class);
$r->getStaticPropertyValue("foo\0bar");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it's possible to have a static property to have a NUL byte?

@DanielEScherzer

Copy link
Copy Markdown
Member Author

I wonder if for a lot of those methods it wouldn't be better to just reject names with nul bytes? As I don't know if anything other than properties can include such a byte?

I figured that kind of refactor shouldn't be done on the older branches - we can definitely switch to using the path ZPP on master after upmerging though

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants