Backd contest - kenta's results

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/oracles/ChainlinkUsdWrapper.sol#L64 https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/oracles/ChainlinkUsdWrapper.sol#L55 https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/oracles/ChainlinkOracleProvider.sol#L55

Vulnerability details

Impact

In ChainlinkUsdWrapper there are no validations for answer(the price) if the price is 0 or not. I checked _ethOracle(0x5f4eC3Df9cbd43714FE2740f5E3616155c5b8419)..latestRoundData(). However, this contract has no validation for the price too. In addition to that, the price feed must be checked if the data is really updated or not.

In ChainlinkOracleProvider answeredInRound >= roundId must be checked too.

Proof of Concept

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/oracles/ChainlinkUsdWrapper.sol#L55 https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/oracles/ChainlinkUsdWrapper.sol#L64 https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/oracles/ChainlinkOracleProvider.sol#L55

Tools Used

code review

Recommended Mitigation Steps

require(answer > 0, "error message"); require(answeredInRound >= roundId, "error message"); require(updatedAt > 0, "error message");

2022-04-backd

1 delete unnecessary function in Erc20Pool.sol.

The following function returns _getBalanceUnderlying without any other executions. You can use _getBalanceUnderlying.

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/pool/Erc20Pool.sol#L42-L44

Delete the function.

##2 use call instead of transfer to transfer ETH in _doTransferOut.

To transfer ETH the use of call is recommended now.

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/pool/EthPool.sol#L30

(bool sent, ) = _to.call{value: amount}(""); require(sent, “error message”);

2022-04-backd optimization

1 use calldata instead of memory.

The following input for function can be calldata instead of memory to save gas.

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/pool/Erc20Pool.sol#L15 https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/pool/EthPool.sol#L13 https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/pool/LiquidityPool.sol#L702 https://github.com/code-423n4/2022-04-backd/blob/main/backd/interfaces/pool/IEthPool.sol#L6 https://github.com/code-423n4/2022-04-backd/blob/main/backd/interfaces/pool/IErc20Pool.sol#L6

string calldata name_,

2 check the following validation at the beginning of updateDepositCap.

The other require statements need to read the state variable. So you can save gas if the input _depositCap is 0 and the execution is reverted.

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

Place the require statement at the beginning of updateDepositCap.

3 use unchecked for the following calculation.

The underflow must happen never because the calculation tries to subtract after balance from before balance.

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/StakerVault.sol#L384

uint256 unstaked; unchecked { unstaked = oldBal - IERC20(token).balanceOf(address(this)); }

4 delete the following local variables that are used only one time in transferFrom.

The local variables allowanceNew, srcTokensNew, and dstTokensNew are used only one time in transferFrom, so you can delete them and set these calculations directly for state variables.

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/StakerVault.sol#L163-L165

balances[src] = srcTokens - amount; balances[dst] = balances[dst] + amount;

if (startingAllowance != type(uint256).max) { _allowances[src][spender] = startingAllowance - amount; }

5 use unchecked for the calculation in decreaseActionLockedBalance.

The underflow is checked already in if sentence, so you can use unchecked to save gas costs.

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/StakerVault.sol#L231

unchecked { actionLockedBalances[account] -= amount; }

6 use the initial value, prefix and unchecked in loop

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/StakerVault.sol#L260 https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/BkdLocker.sol#L310

for (uint256 i; i < length;) { // do something unchecked { ++i; } }

7 use unchecked for the calculation in _withdrawFrom.

The underflow for the following calculation is already checked in the next lines, so you can use unchecked to save gas costs.

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/GasBank.sol#L87

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/GasBank.sol#L68 https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/GasBank.sol#L76

In withdrawUnused the underflow is also checked because the unused amount will be transferred.

unchecked { _balances[account] = currentBalance - amount; }