Align precompile execution with the current call frame - #3130
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| fn execute(&self, handle: &mut impl PrecompileHandle) -> Option<PrecompileResult> { | ||
| match handle.code_address() { | ||
| let code_address = handle.code_address(); | ||
| if !accepts_foreign_frame(code_address) && code_address != handle.context().address { |
There was a problem hiding this comment.
[HIGH] Frame guard intercepts non-precompile delegate calls
PrecompileSet::execute must return None for addresses outside this set. Because this check precedes the address match, any ordinary contract reached through DELEGATECALL or CALLCODE has a mismatched frame and returns Some(Err(...)); the EVM consequently treats it as a handled precompile failure instead of executing the contract. This can disable proxy contracts and make contract-controlled assets inaccessible. First establish that code_address belongs to this precompile set, as Frontier's fragment implementation does, and only then apply the frame restriction.
| if !accepts_foreign_frame(code_address) && code_address != handle.context().address { | |
| if Self::used_addresses().contains(&code_address) | |
| && !accepts_foreign_frame(code_address) | |
| && code_address != handle.context().address | |
| { |
🛡️ AI Review — Skeptic (security review)VERDICT: SAFE MEDIUM scrutiny: 5-month-old account, mitigated by repository write access and substantial merged history; no Gittensor association found. Branch targets main. The frame-alignment guard now covers every currently registered caller-signed precompile while retaining foreign-frame access only for cryptographic and read-only precompiles. No security vulnerability or malicious behavior was found. FindingsNo findings. Prior-comment reconciliation
ConclusionThe previously exposed signed-dispatch paths are now protected against 📜 Previous run (superseded)
🔍 AI Review — Auditor (domain review)VERDICT: 👍 Gittensor association: UNKNOWN; established contributor with repository write access and extensive prior contributions. Description discrepancyThe summary says only stateless cryptographic precompiles may execute in a borrowed frame, while the implementation also permits read-only precompiles. It should say “read-only and cryptographic precompiles.” Quick preflight: FindingsNo findings. Prior-comment reconciliation
ConclusionThe revised allowlist now restricts foreign-frame execution to cryptographic and read-only precompiles, addressing the prior signed-dispatch concern. The implementation and regression coverage are ready to merge. 📜 Previous run (superseded)
|
|
🔄 AI review updated — Skeptic: VULNERABLE |
Co-authored-by: Cursor <cursoragent@cursor.com>
| fn accepts_foreign_frame(address: H160) -> bool { | ||
| const PURE_MATH: &[u64] = &[1, 2, 3, 4, 5, 7, 8, 9, 1024, 1025]; | ||
| PURE_MATH.iter().any(|&index| address == hash(index)) | ||
| || address == hash(Ed25519Verify::<[u8; 32]>::INDEX) |
There was a problem hiding this comment.
[MEDIUM] Test the foreign-frame exception path
The tests cover rejection of a stateful precompile, but none execute an allowlisted cryptographic precompile with code_address != context.address. Add a test for at least one standard cryptographic precompile and one signature-verification precompile so an address-list typo cannot silently break the documented DELEGATECALL compatibility.
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
| fn requires_direct_call(address: H160) -> bool { | ||
| address == hash(6) | ||
| || address == hash(BalanceTransferPrecompile::<R>::INDEX) | ||
| || address == hash(StakingPrecompile::<R>::INDEX) | ||
| || address == hash(StakingPrecompileV2::<R>::INDEX) | ||
| } | ||
|
|
||
| pub fn used_addresses() -> [H160; 33] { |
There was a problem hiding this comment.
[HIGH] Protect every signed-dispatch precompile
This list omits Subnet, Neuron, Alpha, Crowdloan, Leasing, VotingPower, Proxy, and Balance, although each can dispatch a runtime call using an origin derived from context.caller. A contract reached by an EOA can therefore use a foreign frame to invoke those operations as that EOA—the same confused-deputy path this check is intended to close. Restrict every precompile that dispatches as the caller; leave foreign-frame execution only for genuinely read-only or cryptographic precompiles.
| fn requires_direct_call(address: H160) -> bool { | |
| address == hash(6) | |
| || address == hash(BalanceTransferPrecompile::<R>::INDEX) | |
| || address == hash(StakingPrecompile::<R>::INDEX) | |
| || address == hash(StakingPrecompileV2::<R>::INDEX) | |
| } | |
| pub fn used_addresses() -> [H160; 33] { | |
| /// Precompiles that dispatch pallet calls as `context.caller`. | |
| /// Direct CALL only. | |
| fn requires_direct_call(address: H160) -> bool { | |
| address == hash(6) | |
| || address == hash(BalanceTransferPrecompile::<R>::INDEX) | |
| || address == hash(StakingPrecompile::<R>::INDEX) | |
| || address == hash(StakingPrecompileV2::<R>::INDEX) | |
| || address == hash(SubnetPrecompile::<R>::INDEX) | |
| || address == hash(NeuronPrecompile::<R>::INDEX) | |
| || address == hash(AlphaPrecompile::<R>::INDEX) | |
| || address == hash(CrowdloanPrecompile::<R>::INDEX) | |
| || address == hash(LeasingPrecompile::<R>::INDEX) | |
| || address == hash(VotingPowerPrecompile::<R>::INDEX) | |
| || address == hash(ProxyPrecompile::<R>::INDEX) | |
| || address == hash(BalancePrecompile::<R>::INDEX) | |
| } |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
| /// Dispatch (`0x06`) signs pallet calls as `context.caller`. Direct CALL only. | ||
| fn requires_direct_call(address: H160) -> bool { | ||
| address == hash(6) |
There was a problem hiding this comment.
[HIGH] Protect every caller-signed precompile
Restricting this guard to address 0x06 leaves other state-mutating precompiles—including balance transfer, staking, subnet, neuron, alpha, crowdloan, leasing, voting-power, proxy, and balance operations—able to dispatch runtime calls as context.caller from a foreign frame. A contract reached by a user can invoke these through DELEGATECALL/CALLCODE, causing the runtime dispatch to be signed as that user. Require a matching frame for every precompile that derives authority from context.caller, while exempting only genuinely read-only or cryptographic precompiles.
|
🔄 AI review updated — Skeptic: VULNERABLE |
Co-authored-by: Cursor <cursoragent@cursor.com>
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
Summary
code_addressto match the current EVM frame address beforeexecute, except for stateless cryptographic precompiles.Test plan
cargo test -p subtensor-precompiles precompile_set_Made with Cursor