Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (2)
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
bcca8f0 to
bcd0a61
Compare
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: bcd0a6162c
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
| ret.heap_pointer |= ret_prov.may_reference_heap(); | ||
| ret.unknown_pointer |= ret_prov.may_reference_unknown_local() |
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
When full-verified IR returns a malloc-tainted value whose arithmetic can leave the heap, do not propagate it as heap-only. Provenance unions malloc bases through arbitrary arithmetic, so heap_pointer becomes true even when the numeric result is msize-32 (a caller alloca). The caller can store that result in another alloca; may_reference_unknown_local() is false, both objects reuse one slot, and the second store overwrites the first. The Revm reproducer returns 160 instead of 48879 at O0; the parent returns 48879. Mark arithmetic-derived results unknown-local unless the operation proves heap confinement, and add this regression.
Useful? React with 👍 / 👎.
bcd0a61 to
6c7a2d0
Compare
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 6c7a2d08e8
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
| .value_ty(value) | ||
| .is_pointer(self.scan_ctx.module) | ||
| && ret_prov.is_empty()); |
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
When an i256 callee can return either a malloc-derived address or a raw numeric address, mark the raw alternative unknown too. This code sets unknown_pointer only for already-unknown provenance or pointer-typed empty values, although i256 is a pointer carrier. A Full-verifier Revm test returning malloc or 160 stores the result through a local slot; this commit returns 160 instead of 48879, while the parent returns 48879. Unlike the existing arithmetic case, this uses a direct raw-i256 return branch. Use type_can_carry_pointer_provenance for empty alternatives and add the regression.
Useful? React with 👍 / 👎.
Distinguish a heap pointer returned by a callee from a pointer that may reference arbitrary caller-local storage. Carry that distinction through forwarding calls and mixed return paths.
The caller cannot assign a local allocation identity to a callee-owned heap object, but that uncertainty should not make every local alloca reachable. Preserving heap origin avoids unnecessarily extending local object lifetimes while keeping heap escape, reclamation, clobbers, and genuinely unknown pointers conservative.
Stacked on #391.