code-423n4 / 2021-10-ambire-findings

0 stars 0 forks source link

`Zapper.sol#tradeV3Single()` Remove unnecessary variable can make the code simpler and save gas #28

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

At L149, params.recipient is read and put into a local variable recipient. However, recipient is only read once when wrapOutputToLending is true. Thus, the variable recipient is unnecessary.

https://github.com/code-423n4/2021-10-ambire/blob/bc01af4df3f70d1629c4e22a72c19e6a814db70d/contracts/wallet/Zapper.sol#L147-L159

function tradeV3Single(ISwapRouter uniV3Router, ISwapRouter.ExactInputSingleParams calldata params, bool wrapOutputToLending) external returns (uint) {
    ISwapRouter.ExactInputSingleParams memory tradeParams = params;
    address recipient = params.recipient;
    if(wrapOutputToLending) {
        tradeParams.recipient = address(this);
    }

    uint amountOut = uniV3Router.exactInputSingle(tradeParams);
    if(wrapOutputToLending) {
        lendingPool.deposit(params.tokenOut, amountOut, recipient, aaveRefCode);
    }
    return amountOut;
}

Recommendation

Change to:

function tradeV3Single(ISwapRouter uniV3Router, ISwapRouter.ExactInputSingleParams calldata params, bool wrapOutputToLending) external returns (uint) {
    ISwapRouter.ExactInputSingleParams memory tradeParams = params;
    if(wrapOutputToLending) {
        tradeParams.recipient = address(this);
    }

    uint amountOut = uniV3Router.exactInputSingle(tradeParams);
    if(wrapOutputToLending) {
        lendingPool.deposit(params.tokenOut, amountOut, params.recipient, aaveRefCode);
    }
    return amountOut;
}
Ivshti commented 2 years ago

resolved in https://github.com/AmbireTech/adex-protocol-eth/commit/ad56b80a8483ff3a03cd9b326c242a4b6162eeec

GalloDaSballo commented 2 years ago

The sponsor has removed the extra declaration