FloorDAO / floor-v2

Floor aims to create a fully onchain governance mechanism for sweeping and deploying NFTs to profitable NFT-Fi strategies as well as seeding liquidity for its own NFT-Fi products.
https://floor.xyz
2 stars 0 forks source link

[UVS-03M] Improper Order of Operations #110

Closed tomwade closed 1 year ago

tomwade commented 1 year ago

UVS-03M: Improper Order of Operations

Type Severity Location
Language Specific UniswapV3Strategy.sol:L137-L138, L141-L142

Description:

The UniswapV3Strategy::deposit function does not conform to the Checks-Effects-Interactions pattern as it will perform a potentially re-entrant call before eliminating the token approvals to the position manager.

Impact:

The referenced approval operations cannot be taken advantage of as the contract does not possess token0 / token1 values at rest. In any case, we advise the statements to be re-ordered as a matter of convention and adherence to standard security practices.

Example:

// Send leftovers back to the caller
params.token0.withdrawTokens(msg.sender, amount0Desired - amount0);
params.token1.withdrawTokens(msg.sender, amount1Desired - amount1);

// Remove approvals
params.token0.approveToken(params.positionManager, 0);
params.token1.approveToken(params.positionManager, 0);

Recommendation:

We advise the code to re-order its statements, erasing any approvals of the token0 and token1 implementations and then withdrawing the tokens to the msg.sender.