code-423n4 / 2021-11-badgerzaps-findings

0 stars 0 forks source link

Avoid unnecessary code execution can save gas #41

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

https://github.com/Badger-Finance/ibbtc/blob/d8b95e8d145eb196ba20033267a9ba43a17be02c/contracts/Zap.sol#L304-L318

function ibbtcToCurveLP(uint poolId, uint bBtc) public view returns(uint lp, uint fee) {
    uint sett;
    uint max;
    (sett,fee,max) = settPeak.calcRedeem(poolId, bBtc);
    Pool memory pool = pools[poolId];
    if (bBtc > max) {
        return (0,fee);
    } else {
        // pesimistically charge 0.5% on the withdrawal.
        // Actual fee might be lesser if the vault keeps keeps a buffer
        uint strategyFee = sett.mul(controller.strategies(pool.lpToken).withdrawalFee()).div(10000);
        lp = sett.sub(strategyFee).mul(pool.sett.getPricePerFullShare()).div(1e18);
        fee = fee.add(strategyFee);
    }
}

L309-311 is necessary as they won't affect the storage or the returns anyway.

Recommendation

Change to:

if (bBtc <= max) {
    // pesimistically charge 0.5% on the withdrawal.
    // Actual fee might be lesser if the vault keeps keeps a buffer
    uint strategyFee = sett.mul(controller.strategies(pool.lpToken).withdrawalFee()).div(10000);
    lp = sett.sub(strategyFee).mul(pool.sett.getPricePerFullShare()).div(1e18);
    fee = fee.add(strategyFee);
}
GalloDaSballo commented 2 years ago

Personally I like the return early, also like the idea presented by the warden, ambivalent on applying the improvement