Delay notice emission until end of opcode

[nikic]

This is a prototype for fixing a long-standing source of interrupt
vulnerabilities: A notice is emitted during execution of an opcode,
resulting in an error handling being run. The error handler modifies
some data structure the opcode is working on, resulting in UAF or
other memory corruption.

The idea here is to instead collect notices and only process them
after the opcode. This is implemented similarly to exception
handling, by switching to a ZEND_HANDLE_DELAYED_ERROR opcode,
which will then switch back to the normal opcode stream.

Unfortunately, what this prototype implements is not sufficient.
Opcodes that acquire direct (INDIRECT) references to zvals require
that no interrupts occur between the producing and the consuming
opcode. Chains of W/RW opcodes should be executed without interrupt.
Currently, the notice is only delayed until after the first opcode,
which still results in an illegal interrupt (bug78598.phpt shows
a UAF with this change).

I'm not sure how to best handle that issue.

[trowski]

Which opcodes require that there are no interruptions? Is there a flag set on them in zend_vm_opcodes_flags (or could there be)?

[nikic]

Which opcodes require that there are no interruptions? Is there a flag set on them in zend_vm_opcodes_flags (or could there be)?

The _W, _RW, and similar opcodes. The problem isn't identifying them, but hijacking control flow at the right place. Replacing the current opline with HANDLE_DELAYED_ERROR works fine if you want notices to be handled directly after the current opcode -- but I'm not sure how to achieve the same if we want to handle it a few opcodes later instead.

[trowski]

Are the _W and _RW opcodes always guaranteed to be followed by another opcode? If so, incrementing past those opcodes and replacing the next wound seem fine, no?

[nikic]

@trowski It's not possible to modify the opcode stream (it is usually in immutable in SHM). The only thing we can do is change the pointer to the currently executed instruction.

[trowski]

Ah yes, I assumed there was a reason the simple solution wouldn't work, thanks for the explanation.

You could create a temporary op_array that is pushed on the stack containing only the uninterruptible opcodes and the error handler, then free the temp op_array either in the error handler or in another opcode. I'm sure you thought of that too and there are complications to that solution. 😆

[iluuu1994]

Chains of W/RW opcodes should be executed without interrupt.

I'm looking into a solution for this. One approach may be to set EG(delayed_error_op)[0] to the next opline if the warning cannot be handled immediately. After executing this opline, advancement to the next opline will trigger ZEND_HANDLE_DELAYED_ERROR again. Once we reach an interruptible opcode the warning can be emitted.

One issue is that RT_CONSTANT inside the assign handlers will break, given that is is relative to the current opline. We may check in the assign handlers whether opline == &EG(delayed_error_op)[0], and then use EG(opline_before_exception) for fetching the constant instead. If this leads to a performance regression, we may try to create a specialization that can be used for this case only.

There may be more issues that I haven't discovered yet.

[dstogov]

I think this outdated PR should be closed.