sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

Chom - Unexpected behavior for UniV2Adapter, UniV3Adapter, and ZeroExAdapter when msgValue is not zero #122

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Chom

medium

Unexpected behavior for UniV2Adapter, UniV3Adapter, and ZeroExAdapter when msgValue is not zero

Summary

Unexpected behavior for UniV2Adapter, UniV3Adapter, and ZeroExAdapter when msgValue is not zero.

Vulnerability Detail

UniV2Adapter, UniV3Adapter, and ZeroExAdapter are assuming msgValue is zero. But the caller logic is shared between BalancerV2Adapter and CurveAdapter which support positive msgValue. So, there is a chance that UniV2Adapter, UniV3Adapter, and ZeroExAdapter will be given positive msgValue. Since they don't handled positive msgValue cases, it will return an unexpected behavior.

Impact

If msgValue is not zero, UniV2Adapter, UniV3Adapter, and ZeroExAdapter may execute unexpected behavior. For example, UniV2Adapter may swap WETH to tokens instead of swapping native ETH to tokens.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/adapters/UniV2Adapter.sol#L12-L52

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/adapters/UniV3Adapter.sol#L67-L88

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/adapters/ZeroExAdapter.sol#L12-L24

Tool used

Manual Review

Recommendation

Revert when msgValue is not zero

 function getExecutionData(address from, Trade calldata trade) 
     internal view returns ( 
         address spender, 
         address target, 
         uint256 msgValue, 
         bytes memory executionCallData 
     ) 
 {
   ...
   require(msgValue == 0, "Positive msgValue");
   ...
 }

Duplicate of #33