Backd contest - fatherOfBlocks's results

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/StrategySwapper.sol#L38-L43 https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/StrategySwapper.sol#L109-L114

Vulnerability details

Impact

in the setSlippageTolerance(L119) method you have certain requirements to set slippageTolerance, but in the constructor you don't.

Recommended Mitigation Steps

I would add the corresponding validations to the constructor

contracts/pool/LiquidityPool.sol

• L169 - In the handleLpTokenTransfer() function it would be correct to validate that the amount != 0.

contracts/strategies/ConvexStrategyBase.sol

• L107-109 - it is not safe to approve for the total, it is preferable to approve for the amounts that are needed.

contracts/CvxCrvRewardsLocker.sol

• L161 - GO - In the withdraw() function, the "balance" variable is only used in one place, it could simply be called on the line it is used.

• L231 - GO - In the unstakeCvxCrv() function, the "staked" variable is only used in one place, it could simply be called on the line it is used.

contracts/GasBank.sol

• L48 - GO - can be wrapped with unckecked, since the if validates that there is no underflow.

contracts/Controller.sol

• L114 - GO - if you don't set totalEthRequired = 0, you save 36 gas.

contracts/StakerVault.sol

• L185 - GO - It wastes less gas if instead of creating a msg.sender variable, you simply use msg.sender in _allowances[msg.sender][] and in the event that is emitted.

• L231 - GO - can be wrapped with unckecked, since the if validates that there is no underflow.

• L331 - GO - oldBal is only used in one place, it could simply be called on the line that is used

contracts/pool/LiquidityPool.sol

• L446 - GO - can be wrapped with unckecked, since lpBalance_ < redeemLpTokens validates that there is no underflow.

• L556 - GO - can be wrapped with unckecked, redeemLpTokens - withdrawalFee can be invalid since to obtain the withdrawalFee it is a percentage of what you want to redeem.

• L633 - GO - In the totalUnderlying() function it is better not to create a variable in memory for _getBalanceUnderlying(), just execute it in the two returns L637 and 640.

• L748.751.753 - GO - the operation that ends by setting the variable in memory requiredDeposits. could be unckecked by the validations that were performed in the if.

contracts/actions/topup/TopUpKeeperHelper.sol

• L43.46.72.93.165 - GO - for i++ unckecked

• L164 - GO - if shortened is defined in the returns of the signature and push is used to add elements, the gas cost drops by 30.