code423n4 commented 1 year ago

Lines of code

https://github.com/prepo-io/prepo-monorepo/blob/3541bc704ab185a969f300e96e2f744a572a3640/apps/smart-contracts/core/contracts/Collateral.sol#L57

Vulnerability details

Description

Collateral's deposit() function is in charge of supplying underlying and minting collateral tokens.

function deposit(address _recipient, uint256 _amount) external override nonReentrant returns (uint256) {
  uint256 _fee = (_amount * depositFee) / FEE_DENOMINATOR;
  if (depositFee > 0) { require(_fee > 0, "fee = 0"); }
  else { require(_amount > 0, "amount = 0"); }
  baseToken.transferFrom(msg.sender, address(this), _amount);
  uint256 _amountAfterFee = _amount - _fee;
  if (address(depositHook) != address(0)) {
    baseToken.approve(address(depositHook), _fee);
    depositHook.hook(_recipient, _amount, _amountAfterFee);
    baseToken.approve(address(depositHook), 0);
  }
  /// Converts amount after fee from base token units to collateral token units.
  uint256 _collateralMintAmount = (_amountAfterFee * 1e18) / baseTokenDenominator;
  _mint(_recipient, _collateralMintAmount);
  emit Deposit(_recipient, _amountAfterFee, _fee);
  return _collateralMintAmount;
}

Note the calculation of mint amount: uint256 _collateralMintAmount = (_amountAfterFee * 1e18) / baseTokenDenominator;

baseTokenDenominator is defined in the constructor: baseTokenDenominator = 10**_newBaseTokenDecimals;

In the event the base token has over 18 decimals, the deposit() funciton is vulnerable to rounding error. For example, the popular NEAR token has 24 decimals. If user supplies 10**5 NEAR tokens, the following will occur: collateralMintAmount = 10**5 * 10**18 / 10**24 = 0 There is no check for rounding to zero and deposit() will simply swallow the 10**5 tokens supplied by user.

Impact

User's supplied tokens may be swallowed and no collateral tokens given when using underlying ERC20 with above 18 decimals

Tools Used

Manual audit

Recommended Mitigation Steps

Add a require statement to make sure the collateralMintAmount > 0.

Picodes commented 1 year ago

Assuming the collateralMintAmount > 0 requirement is implemented, due to the amount being rounded down, amounts of 1e5 can still be "swallowed" by the contract if you deposit say 1e24 + 1e5.

To me this isn't an issue but a logic consequence of rounding down, so is a best practice at best

c4-judge commented 1 year ago

Duplicate of https://github.com/code-423n4/2022-12-prepo-findings/issues/315

code-423n4 / 2022-12-prepo-findings