Delayed effects 2#213
Open
iluuu1994 wants to merge 26 commits into
Open
Conversation
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.
There are 3 categories of failing tests here: - Many are throwing in a user error handlers, therefore relied on non-delayed behavior. Fixed these by installing the error handler with promote_to_exception: true. - Some are just printing the error message. Their output changes due to delay. These are fixed by updating the EXPECT section. - And finally, many are testing the adverse effects of mutations in error handlers. These are now irrelevant, but these are also fixed by updating the EXPECT section.
…_call_frame slow path
|
Important Review skippedToo many files! This PR contains 244 files, which is 94 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (244)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Owner
Author
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
Owner
Author
|
@CodeRabbit review --dir Zend |
|
Tip For best results, initiate chat on the files or code changes.
If you'd like to focus reviews on the reviews:
path_instructions:
- path: "Zend/**"
instructions: "Focus the review on this directory."You can find more details in the path-based instructions documentation. In the meantime, shall I trigger a full review of the PR? |
Owner
Author
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.