fix: fixes vuln in Multisig Plugin #338
Merged
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.
Description
Code4rena finding: code-423n4/2023-03-aragon-findings#140
When the Multisig settings and addresses are updated and in the same block another proposal gets created an attacker could trigger a race condition:
Consider the addresses 0x01, 0x02, 0x03 to be a member of the Multisig, and the minApprovals is set to 2 during block 1.
Now 0x03 is considered a bad actor and a proposal is created and approved by 0x01 and 0x02 during block 2 but not yet executed to remove 0x03 from the member list and reduce minApprovals to 1.
0x03 can now swoop in with a MEV sandwich block 3 and execute the proposal and create a new proposal in the same block and auto-execute it resulting in 0x03 taking over the Multisig.
This problem is because
createProposal()
uses the members list from the previous block (2) but the new settings from block 3 resulting in 0x03 still being a member of the Multisig but having already minApprovals set to 1. So 0x03 can create any proposal, approve it and execute it in the same TX.Type of change
Checklist:
CHANGELOG.md
file in the root folder.