Golom contest - Limbooo's results

Lines of code

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L389-L394 https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L396-L399

Vulnerability details

Functions GolomTrader.sol#fillBid(...) and GolomTrader.sol#fillCriteriaBid(...) used to fill a signed order (Golom order.orderType = 1 or = 2) by the buyer after he approved GolomTrader contract some wETH. However, in case that the signed order is of type ERC1155 where the value of order.isERC721 = false and functions called with param; amount > 1 this issue will occurs. At the end of both functions an internal function _settleBalances(...) called where the issue is handled. Lines | contracts/core/GolomTrader.sol#fillBid(...) | contracts/core/GolomTrader.sol#fillCriteriaBid(...) -> contracts/core/GolomTrader.sol#_settleBalances(...)

Impact

Token owner who try to sell his tokens will get less than what he should be getting and the rest of the ether will stuck forever in the GolomTrader contract since there is no ether rescue function.

Tools Used

vscode, hardhat, manual review.

Proof of Concept

There are more than one arithmetic operations to classify this issue. One is when calculating the protocolfee which we multiply by amount and used when transferring to the RewardDistributor contract and this is what it meant to be. However, in the line GolomTrader.sol#L393 and GolomTrader.sol#L400, protocolfee has been used as its the base fee of the order total amount order.totalAmt which it's not. Thus it used to subtract the total amount of 1 token and then multiply by the amount. At the end the token seller will get less than what he should get, and the amount of ether has been lost will live forever in the contract. The amount of lost ether scale exponentially upon amount entered by user (Token seller).

contracts/core/GolomTrader.sol:
  375      function _settleBalances(
  376          Order calldata o,
  377          uint256 amount,
  378          address referrer,
  379          Payment calldata p
  380      ) internal {
  381:         uint256 protocolfee = ((o.totalAmt * 50) / 10000) * amount;
  ...
  389              payEther(
  390:                 (o.totalAmt - protocolfee - o.exchange.paymentAmt - o.prePayment.paymentAmt - o.refererrAmt) * // @audit
  391                      amount -
  392                      p.paymentAmt,
  393                  msg.sender
  394              );
  ...
  396              payEther(
  397:                 (o.totalAmt - protocolfee - o.exchange.paymentAmt - o.prePayment.paymentAmt) * amount - p.paymentAmt, // @audit
  398                  msg.sender
  399              );
  ...
  403      }

Recommended Mitigation Steps

subtract the base protocol fee before its scaled to total fee amount, or subtract it like p.paymentAmt after multiplied by amount.

(o.totalAmt - o.exchange.paymentAmt - o.prePayment.paymentAmt - o.refererrAmt) *  amount - p.paymentAmt - protocolfee,

• contracts/core/GolomTrader.sol#L392-L397
• contracts/core/GolomTrader.sol#L399-L402