Skip to content

streams: Fix UAF when a user wrapper unregisters itself - #24754

Merged
iliaal merged 1 commit into
php:masterfrom
iliaal:fix/user-wrapper-url-ops-uaf
Nov 1, 2026
Merged

iliaal merged 1 commit into
php:masterfrom
iliaal:fix/user-wrapper-url-ops-uaf

Conversation

@iliaal

@iliaal iliaal commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

A user stream wrapper whose constructor, destructor or __set() calls stream_wrapper_unregister() frees the wrapper while unlink(), rename(), mkdir(), rmdir(), touch(), chmod() or stat() is still using it; the constructor lookup and the "is not implemented!" warning then read the freed wrapper. These operations now read the class entry before any user code runs, user_stream_create_object() takes the class entry instead of the wrapper, and the warning is reported through php_stream_wrapper_warn_name(). Found while reviewing #24711, which fixes failed opendir() handling on PHP-8.6.

@ndossche ndossche 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.

Prior to the new stream APIs it would've been easy to hold onto uwrap->ce and then we didn't need all this refcount juggling.
It's still possible to do this and then use php_stream_wrapper_warn_name...
What do you think?

A user wrapper's constructor, destructor or __set() can unregister the
wrapper while unlink(), rename(), mkdir(), rmdir(), touch(), chmod() or
stat() is using it, which frees the wrapper, and the code then reads the
freed wrapper to find the class constructor and to build the "is not
implemented!" warning. Read the class entry before any user code runs,
pass it to user_stream_create_object(), and report the warning by the
user-space wrapper name.
@iliaal
iliaal force-pushed the fix/user-wrapper-url-ops-uaf branch from 1421ebc to 406166f Compare November 1, 2026 15:56
@iliaal iliaal changed the title streams: Keep user wrappers alive across URL operations streams: Fix UAF when a user wrapper unregisters itself Nov 1, 2026
@iliaal

iliaal commented Nov 1, 2026

Copy link
Copy Markdown
Member Author

Agreed, reworked that way. user_stream_create_object() now takes the class entry as well, since __set() on the context property can unregister the wrapper before its constructor lookup. The test covers that case.

@ndossche ndossche 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.

Thanks.
I like this approach better for two reasons:

  • It's simpler
  • No refcount juggling

@iliaal
iliaal merged commit f4d80ce into php:master Nov 1, 2026
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants