Repository navigation
Withdraw context words read their own column or revert - #2927
Merged
Merged
Conversation
withdraw4 post tasks get the withdraw values (plus msg.sender as the withdrawer row) in a dedicated calling context column 6; every other column is empty, so withdraw words read outside withdraw tasks revert out of bounds. Signers move to column 7 and signed context to 8. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…text-column-withdraw
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 4 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Oct 26, 2026
Contributor
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
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
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.
PR 2 of 4, based on
context-column-deposit(#2926). Split from #2921 (one context column per PR). Advances #2647.Stack: PR1 deposit column -> PR2 withdraw column (this) -> PR3 order column -> PR4 signed-context row mask.
Problem
The withdraw post task wrote its data into calling-context column 1, the same column order words read. So
order-hash()in a withdraw task silently returned the withdraw token, andwithdraw-*words in an add-order task silently returned order data (#2647).withdrawer()aliased base(0,0)and read a value in every entrypoint.Fix
CALLING_CONTEXT_COLUMNS = 6; newCONTEXT_WITHDRAW_COLUMN = 6, filled only bywithdraw4post:[token, vaultId, before, after, target, decimals, msg.sender]viaLibRaindex.callingContextColumn.withdraw-*words read column 6;withdrawerreads(6,6)(newsubParserWithdrawer).CONTEXT_COLUMNS);CONTEXT_COLUMNS_EXTENDED = CONTEXT_SIGNED_CONTEXT_START_COLUMN + 1; withdraw tables sit atCONTEXT_WITHDRAW_COLUMN.order-cleareris unchanged here (PR3).Why
Same mechanism as PR1: each entrypoint fills only its own columns, so a withdraw word read anywhere else panics out of bounds instead of reading another entrypoint's value. Full design rationale: #2921.
Evidence
RaindexV6.contextWordEntrypoints.t.sol: deposit- and withdraw-column words (withdraw-token,withdraw-vault-id,withdraw-vault-balance-before/after,withdraw-target-amount,withdraw-token-decimals,withdrawer) x {deposit4, withdraw4, addOrder4, removeOrder3, entask2, calculate-io, handle-io}: true value in their family,stdError.indexOOBErrorelsewhere. Calculate-io reads go throughquote2(since takeOrders4 skips an order whose calculate fails #2923takeOrders4skips a reverting calculate).forge test: on 48f8128: 749 passed / 3 failed (752); the 3 are env/network (testIsStartBlockFlare,testSuitesLiveOnEverySupportedNetwork,testSupportedNetworkChainIdsAreBound, noFLARE_RPC_URL).QA
RaindexV6ContextWordEntrypointsTest(testWithdrawContextWords,testDepositContextWords,testAddOrderContextWords,testRemoveOrderContextWords,testEntaskContextWords,testCalculateOrderContextWords,testHandleIOContextWords) over the withdraw-column words,testRouteWithdrawer. Fail on base (PR1): there withdraw words read column 1 (a wrong-entrypoint read returns order data instead of reverting) andwithdrawerreads base(0,0).msg.sendervia prank, token, vault id, amounts), independent of the sub parser andLibRaindex.Migration
context<6 i>/context<7+i j>->context<7 i>/context<8+i j>.context<1 n>->context<6 n>.🤖 Generated with Claude Code