QA Report

[code423n4]

[low-1] Lack of accuracy upon small amount on fee calculation.

In solidity division can be zero; 9 / 10 = 0. However, all values in (order.strike * fee) / 1000 where (order.strike * fee) < 1000 will be unsettled for fee payment and would just transfer a zero value to the owner of the contract. (999 * 1) / 1000 = 0 where 1 is the fee of 0.01%. Imagine that one day the ether has gone above $1,000,000 or the ERC20(order.baseAsset) used has less decimals by nature with high value, the owner of the contract would not be able to get any profit which the fee calculation required to do.

PoC

In PuttyV2.sol#withdraw()#L499 the division happened and transfering the fee amount immediately:

src/PuttyV2.sol:
497 uint256 feeAmount = 0;
498 if (fee > 0) {
499: feeAmount = (order.strike * fee) / 1000;
500 ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);
501 }

Recommendation

Use FixedPoint Arithmetic library or scale up amount before division to a minimum upon divider value, and scale down after the division. At least, if this not in your concern; skip the transfer if feeAmount == 0.

[non-2] Ignoring function return may affect user funds

When using transfer with WETH we need to assert the call so we make sure the transfer process completed successfully.

PoC

• PuttyV2.sol#fillOrder()#L336

  src/PuttyV2.sol:
  335 IWETH(weth).deposit{value: msg.value}();
  336: IWETH(weth).transfer(order.maker, msg.value);

  Recommendation

  Use assertion to revert on failure

  assert(IWETH(weth).transfer(order.maker, msg.value));

[HickupHH3]

[low-1] isn't upgraded because it lacks the key step of linking 0 fee to bricking withdrawals