code-423n4 / 2022-01-yield-findings

1 stars 0 forks source link

ConvexYieldWrapper wrap can be front-run #134

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

hyh

Vulnerability details

Impact

Now wrap operate with tokens that were sent to the contract before, expecting a user to deal with any front running issues.

If a user will not make actual token transfer and wrap atomic, i.e. will not run them from an another contract within one transaction, an attacker can monitor first token transfer call and front run the second wrap call, effectively stealing all the user's tokens previously sent to the contract.

Proof of Concept

wrap use tokens that already sit on contract balance, not ensuring that transfer from a user, _mint, and transfer to a user is done within one transaction:

https://github.com/code-423n4/2022-01-yield/blob/main/contracts/ConvexYieldWrapper.sol#L122-150

Recommended Mitigation Steps

Consider using safeTransferFrom mechanics within the functions instead of expecting tokens to be on the contract balance

devtooligan commented 2 years ago

It is the user’s responsibility to transfer in tokens and call wrap() within the same atomic transaction. This is a common pattern found within Yield as detailed in the readme

alcueca commented 2 years ago

Duplicate of #87

GalloDaSballo commented 2 years ago

As described in the Readme, the contract is used for operations and should be interacted with with other contracts