Kuiper contest - defsec's results

Handle

defsec

Vulnerability details

Impact

During the manual code review, It has been observed that zero value has not been checked on that "ibRatio" variable. That can cause miscalculation of the liquidity.

Proof of Concept

1. Navigate to "https://github.com/code-423n4/2021-09-defiProtocol/blob/main/contracts/contracts/Basket.sol"
2. Go to the line #217.

""" ibRatio = newRatio; """

3. Onlyauction modifier can assign ibRation to 0.
4. That can affect tokenAmount on the function.

""" function pushUnderlying(uint256 amount, address to) private { for (uint256 i = 0; i < weights.length; i++) { uint256 tokenAmount = amount * weights[i] * ibRatio / BASE / BASE; IERC20(tokens[i]).safeTransfer(to, tokenAmount); } } """

Tools Used

None

Recommended Mitigation Steps

Validate to ibRatio variable is more than zero.

""" require(ibRation > 0 , "ibRatio should be more than zero"); """

Handle

defsec

Vulnerability details

Impact

This is probably an oversight since SafeERC20 was imported and safeTransfer() was used for ERC20 token transfers. Nevertheless, note that approve() will fail for certain token implementations that do not return a boolean value (). Hence it is recommend to use safeApprove().

Proof of Concept

1. Navigate to "https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol".
2. Preview approveUnderlying function on the contract.
3. approve function has been used instead of SafeApprove.

Tools Used

Manual Code Review

Recommended Mitigation Steps

Update to _token.safeApprove(spender, type(uint256).max) in approveUnderlying().