Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(protocol): check proposer param must be msg.sender in PreconfRouter #18851

Closed
wants to merge 4 commits into from

Conversation

dantaik
Copy link
Contributor

@dantaik dantaik commented Jan 30, 2025

This was reported by OZ. Note that same issue also exist in our main branch, but the code will not be used in production for Hekla/Mainnet.

Possible to Debit Bond Balances from Any Account
Taiko developed the PreconfTaskManager and related smart contracts to address preconfirmation challenges in based rollups. In this approach, users that want to execute transaction pass them to a proposer, who provides a preconfirmation that can be used to penalize the proposer if the transaction is not included. At the start of each epoch, a proposer is designated for the next epoch. Once the PreconfTaskManager is deployed and the appropriate name is assigned in the resolver, the designated proposer calls the newBlockProposals function, triggering a block proposal in TaikoL1 after stake locking and other parameter validations. Notably, there is no validation of the blockParamsArrargument in this workflow.
In the TaikoL1 contract, LibProposing’s proposeBlocks function manages block proposals by calling _proposeBlock for each proposed block. After verifying the block number, it retrieves the preconfTaskManager address. If that address is set, only PreconfTaskManager is allowed to propose blocks, and the actual proposer is assumed to be encoded in the _params argument. This argument is decoded and checked in _validateParams. If _local.params.proposer is set to a non-zero address, the caller must be the designated proposer or _local.allowCustomProposer must be true (which it is when called through PreconfTaskManager). In case of default values, the contract will compute and assign the correct values on its own. The decoded proposer address is written to the block metadata stored in the contract’s blocks, and the proposer’s bond balance is debited by the livenessBond amount of 125 Taiko tokens. Because any address can be passed as the proposer, an external actor can effectively debit tokens from an arbitrary account. The actor can also provide invalid data in the proposed block to dispute it and potentially claim part of specified proposer’s bond balance.
Consider implementing verification of both the caller and the proposer address in the PreconfTaskManager contract and the LibProposing library to ensure only the genuine proposer’s bond is debited.

@dantaik dantaik marked this pull request as ready for review January 30, 2025 08:15
Copy link

openzeppelin-code bot commented Jan 30, 2025

fix(protocol): check proposer param must be msg.sender in PreconfRouter

Generated at commit: e3c1ed57c0cd8f17aefe70b1bb6f850832f8e6e2

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total
3
3
0
10
40
56
Dependencies Critical
High
Medium
Low
Note
Total
0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector

@dantaik dantaik changed the base branch from pacaya_fork to pacay_fork_fix_bonds January 30, 2025 11:29
@dantaik
Copy link
Contributor Author

dantaik commented Jan 30, 2025

It turns out this "fix" is not necessary as we have already verified the actual proposer must be msg.sender as the last statement:

// Verify that the sender had set itself as the proposer
require(meta_.proposer == msg.sender, ProposerIsNotTheSender());

@dantaik dantaik closed this Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants