search

code-423n4 / 2023-04-frankencoin-findings

5 stars 4 forks source link

`createNewPosition`/`clonePosition`/`createClone` are suspicious of the reorg attack #985

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/PositionFactory.sol#L13 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/PositionFactory.sol#L37 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/PositionFactory.sol#L30

Vulnerability details

Description

The createNewPosition function creates a new position smart contract and returns its address. The address is determined by create address derivation, that depends on the contract nonce.

Later user could use interact with newly created contract.

At the same time, block reorg may happen on any blockchain. A very clear example to consider would be Polygon:

Please note, reorg on Polygon happens really often. Some are 1 block long, some are >5 minutes long. For the latest, it is quite enough to call one or two functions, especially when someone uses a script, and not doing it by hand.

Example, the Polygon had an incident of 5.5 minutes long reorg 2 weeks ago.

Another example, the incident of 4 minutes long reorg back in December.

While Ethereum seems to be stable, there is no guarantee of the absence of reorgs, which happened a lot previously.

Another interesting impact of this is Optimistic rollups that are quite popular nowadays. Network operators can include a transaction in a block, the user receives the block confirmation and makes a transaction, but the operator reorganizes the blocks for the sake of profit (or because of any other reason, like a bug in server implementation).

Users usually don't have a full node of optimistic rollup to validate whether the state can't be reverted, the same as a user doesn't wait 7 days between submitting transactions.

All in all, the block reorg is expected behavior for the blockchain and should handle properly to avoid loss of user funds.

Attack scenario

Imagine that Alice creates a position smart contract by one of the functions createNewPosition/clonePosition/createClone. And Alice Bob sees that the network block reorg happens and calls the same function with different parameters. Thus, it creates position with an address that Alice expected to have. Any Alices’ transactions that are executed to position smart contaract (like create challenge and all related functionality) will be executed on position with Bobs provided parameters.

Impact

Any significant reorg incident creates an opportunity to steal users’ funds. Also, the use of a small number of confirmations in user transactions can lead to a loss of money.

Lastly, if users rely on the address derivation in advance or try to create the position contract with the same address on different EVM chains, any funds sent to the dao could potentially be stolen by anyone else. All in all, it could lead to the theft of funds/special permissions that dao should hold.

Recommended Mitigation Steps

Use create2 instead of create, with salt that depends on msg.sender.

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as primary issue

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as duplicate of #155

c4-judge commented 1 year ago

hansfriese marked the issue as satisfactory