code-423n4 / 2024-07-reserve-findings

1 stars 0 forks source link

RToken depositors can lose funds if their deployment is part of a block reorg #23

Closed c4-bot-9 closed 1 month ago

c4-bot-9 commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/Deployer.sol#L107

Vulnerability details

The Deployer contract deploys RToken components as a set of ERC1967 Proxies, as follows:

File: Deployer.sol
121:         // Components - Proxies
122:         IRToken rToken = IRToken(
123:             address(new ERC1967Proxy(address(_implementations.components.rToken), new bytes(0)))
124:         );
125:         Components memory components = Components({
126:             stRSR: IStRSR(
127:                 address(new ERC1967Proxy(address(_implementations.components.stRSR), new bytes(0)))
128:             ),

This way of creating smart contracts, that is translated to the EVM CREATE opcode, has an unsafe outcome because the created addresses depend on the ordering of creations.

Reorgs of up to 7 slots have taken place post-Merge on mainnet, which is more than enough time for someone to deploy an RToken and deposit funds to it. On L2s, L1 reorgs can cause even deeper L2 reorgs.

Within the contracts in scope, there is significant loss of funds, as shown in the below PoC.

Impact

Users funding RTokens are vulnerable to lose their funds if the transaction that deployed the funded RToken is part of a block reorg.

Proof of Concept

If a chain reorg happens, and the transactions are reordered to 3, 1, 2, Ashley's funds will be directed to Brian's RToken because the StRSR addresses are now inverted from when they were in the original ordering. At this point, if Brian becomes aware of the situation before Ashley does, Brian can use their ownership powers to steal Ashley's tokens, for example by upgrading to a StRSR implementation that grants them infinite balance as shown in this PoC.

Tools Used

Code review, Foundry

Recommended Mitigation Steps

Consider modifying the Deployer to deploy components using CREATE2 instead, possibly with a salt that includes the msg.sender and deployment values.

Assessed type

Other

tbrent commented 1 month ago

Dupe with #24

known issue: https://github.com/reserve-protocol/protocol/blob/72fc1f6e41da01e733c0a7e96cdb8ebb45bf1065/audits/Code4rena%20-%20Reserve%20Audit%20Report%20-%20Release%203.0.0%20(core).md#03-reorg-attack

c4-judge commented 1 month ago

thereksfour marked the issue as unsatisfactory: Invalid