Backd contest - shenwilly's results

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/pool/LiquidityPool.sol#L523

Vulnerability details

Impact

When ERC777 token is used as the underlying token for a LiquidityPool, a depositor can reenter depositFor and bypass the depositCap requirement check, resulting in higher total deposit than intended by governance.

Proof of Concept

• An empty ERC777 liquidity pool is capped at 1.000 token.
• Alice deposits 1.000 token. Before the token is actually sent to the contract, tokensToSend ERC777 hook is called and Alice reenters depositFor.
• As the previous deposit hasn't been taken into account, the reentrancy passes the depositCap check.
• Pool has 2.000 token now, despite the 1.000 deposit cap.

Recommended Mitigation Steps

Add reentrancy guards to depositFor.

Lines of code

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

Vulnerability details

Impact

When setting slippageTolerance, there is a check to ensure that the value is set within reasonable boundary. This check is missing in StrategySwapper constructor, which could cause irregularities when wrong value is used:

• If set higher than ScaledMath.ONE, swaps will always fail.
• If set to a very low number, strategies will lose funds as swaps are vulnerable to sandwich attacks.

Recommended Mitigation Steps

Add the same check in setSlippageTolerance to constructor.

require(slippageTolerance_ <= ScaledMath.ONE, Error.INVALID_SLIPPAGE_TOLERANCE);
require(slippageTolerance_ > 0.8e18, Error.INVALID_SLIPPAGE_TOLERANCE);

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/StrategySwapper.sol#L287-L289 https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/StrategySwapper.sol#L318-L320 https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/StrategySwapper.sol#L335-L337

Vulnerability details

Impact

In StrategySwapper, swapping from or to tokens with decimals higher than 18 will always revert. This will cause inabilities for strategies to harvest rewards.

Proof of Concept

L288 will revert when token_ has higher than 18 decimals.

 return 10**(18 - IERC20Full(token_).decimals());

Recommended Mitigation Steps

Consider modifying how _decimalMultiplier works so it could handle tokens with higher than 18 decimals.

Update the calculation of _minTokenAmountOut and _minWethAmountOut to account when decimals are higher/lower than 18.

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/pool/LiquidityPool.sol#L694

Vulnerability details

Impact

Depositors won't be able to transfer or redeem funds temporarily.

The problem is caused by the implementation of LiquidityPool.getNewCurrentFees:

function getNewCurrentFees(
    uint256 timeToWait,
    uint256 lastActionTimestamp,
    uint256 feeRatio
) public view returns (uint256) {
    uint256 timeElapsed = _getTime() - lastActionTimestamp;
    uint256 minFeePercentage = getMinWithdrawalFee();
    if (timeElapsed >= timeToWait) {
        return minFeePercentage;
    }
    uint256 elapsedShare = timeElapsed.scaledDiv(timeToWait);
    return feeRatio - (feeRatio - minFeePercentage).scaledMul(elapsedShare);
}

The last line requires the current feeRatio to be higher than minFeePercentage or the function will revert. When this condition is broken, some critical functions such as transferring tokens and redeeming will be unusable. Affected users need to wait until enough time has elapsed and getNewCurrentFees returns minFeePercentage on L691.

This could happen if governance changes the MinWithdrawalFee to be higher than a user's feeRatio.

Proof of Concept

• Initial MinWithdrawalFee is set to 0, MaxWithdrawalFee is set to 0.03e18.
• Alice deposits fund and receives LP token. Alice's feeRatio is now set to 0.03e18 (the current MaxWithdrawalFee).
• Governance changes MaxWithdrawalFee to 0.05e18 and MinWithdrawalFee to 0.04e18.
• minFeePercentage is now higher than Alice's feeRatio and she can't transfer nor redeem the LP token until timeElapsed >= timeToWait.

Recommended Mitigation Steps

Add a new condition in getNewCurrentFees L690 to account for this case:

if (timeElapsed >= timeToWait || minFeePercentage > feeRatio) {
    return minFeePercentage;
}