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
Array assignment fails when the array has been resized in error handler #13754
Comments
Hi @m4p1e. Thank you for your report. We are aware of various such issues, but they are unfortunately fundamentally hard to solve for all cases. My latest attempt is here: #12805 Luckily, such issues only happen in code that is seemingly explicitly crafted to cause it. I haven't read your paper yet, but I will spend some time on it. |
Thank you for your prompt response. In my paper, I've stated, "This problem is not destined to be easily resolved." Why do I care about it? I've been deeply involved in a project concerning the PHP sandbox for running untrusted PHP code. This issue fundamentally breaks the sandbox, hence we have to disable the function |
Creating multidimensional DIM opcodes would solve this problem, but not others. $foo->bar['baz'] = $baz; This issue is very similar.
class Foo {
public $bar = [];
}
$foo = new Foo();
set_error_handler(function () {
global $foo;
$foo->bar = null;
});
$foo->bar['baz'] = $baz;
var_dump($foo); Compile php-src with The error handler swaps the zval pointed to by The approach from #12805 tries to delay the emission of warnings to a safe point in time instead. More specifically, this is when the current opcode no longer accepts indirect values. The PR is relatively straight-forward. However, it is unfortunately not compatible with the JIT, and I did not invest more time to try and fix that, because the JIT is extremely complicated and I already invested too much time on this theoretical issue. There are also other challenges, namely manually identifying exactly what warnings need to be delayed. |
You're right. I missed the cases in object assignment and fetch.
I was thinking of exposing all possible warnings as early as possible to make sure they don't affect following operations, which is exactly the opposite of your approach.
I agree. I have closely reviewed your PR, and it is great. In particular, I like the idea of delaying the warnings from sequential fetches until their results have been used.
The reason I'm reporting this issue is also because of JIT. With the rapid development of JIT technology in PHP today, this issue may become increasingly challenging to fix, and in turn, may hinder the progress of JIT technology in PHP. So, I think it should be fixed as soon as possible.
That shouldn't be very difficult to just fix the user error handler-related issues. But if the delayed error is introduced as a more general mechanism in PHP, its implementation needs more discussion. |
That's exactly right. All the problematic opcodes are listed here: 198b22a#diff-773bdb31563a0f907c75068675f6056b25f003e61f46928a31d9837ae107460dR8103-R8113 They can be combined in all kinds of ways, so I believe creating compound opcodes is not a viable option. Feel free to pick this PR back up and try to add JIT support. I won't have time in the immediate future, but I'm happy to discuss/help out where possible. |
Description
The simple reproduction code as follows.
Resulted in this output:
But I expected this output instead:
This has been an issue for years (at least I learned about it 4 years ago). I wrote a paper for exploiting this as a security vulnerability.
PHP Version
PHP 8.3.3
Operating System
Ubuntu 20.04
The text was updated successfully, but these errors were encountered: