code4rena-2023-01-biconomy-l01

[L-01] Prevent division by 0

보고서

Summary

0으로 나누는 것을 명시적으로 확인하고 핸들링해야 한다고 제안했다.

Keyword

division by 0, arithmetic error

Vulnerability

2 results - 1 file
 
contracts/smart-contract-wallet/SmartAccount.sol:
  264:             payment = (gasUsed + baseGas) * (gasPrice) / (tokenGasPriceFactor);
  288:             payment = (gasUsed + baseGas) * (gasPrice) / (tokenGasPriceFactor);
contracts/smart-contract-wallet/SmartAccount.sol:
  246  
  247:     function handlePayment(
  248:         uint256 gasUsed,
  249:         uint256 baseGas,
  250:         uint256 gasPrice,
  251:         uint256 tokenGasPriceFactor,
  252:         address gasToken,
  253:         address payable refundReceiver
  254:     ) private nonReentrant returns (uint256 payment) {
  255:         // uint256 startGas = gasleft();
  256:         // solhint-disable-next-line avoid-tx-origin
  257:         address payable receiver = refundReceiver == address(0) ? payable(tx.origin) : refundReceiver;
  258:         if (gasToken == address(0)) {
  259:             // For ETH we will only adjust the gas price to not be higher than the actual used gas price
  260:             payment = (gasUsed + baseGas) * (gasPrice < tx.gasprice ? gasPrice : tx.gasprice);
  261:             (bool success,) = receiver.call{value: payment}("");
  262:             require(success, "BSA011");
  263:         } else {
  264:             payment = (gasUsed + baseGas) * (gasPrice) / (tokenGasPriceFactor);
  265:             require(transferToken(gasToken, receiver, payment), "BSA012");
  266:         }
  267:         // uint256 requiredGas = startGas - gasleft();
  268:         //console.log("hp %s", requiredGas);
  269:     }
  270: 
  271:     function handlePaymentRevert(
  272:         uint256 gasUsed,
  273:         uint256 baseGas,
  274:         uint256 gasPrice,
  275:         uint256 tokenGasPriceFactor,
  276:         address gasToken,
  277:         address payable refundReceiver
  278:     ) external returns (uint256 payment) {
  279:         uint256 startGas = gasleft();
  280:         // solhint-disable-next-line avoid-tx-origin
  281:         address payable receiver = refundReceiver == address(0) ? payable(tx.origin) : refundReceiver;
  282:         if (gasToken == address(0)) {
  283:             // For ETH we will only adjust the gas price to not be higher than the actual used gas price
  284:             payment = (gasUsed + baseGas) * (gasPrice < tx.gasprice ? gasPrice : tx.gasprice);
  285:             (bool success,) = receiver.call{value: payment}("");
  286:             require(success, "BSA011");
  287:         } else {
  288:             payment = (gasUsed + baseGas) * (gasPrice) / (tokenGasPriceFactor);
  289:             require(transferToken(gasToken, receiver, payment), "BSA012");
  290:         }
  291:         uint256 requiredGas = startGas - gasleft();
  292:         //console.log("hpr %s", requiredGas);
  293:         // Convert response to string and return via error message
  294:         revert(string(abi.encodePacked(requiredGas)));
  295:     }

0으로 나누려고 하는지 명시적으로 확인하지 않았다.

Impact

0으로 나누어 컨트랙트가 오류로 revert 된다.

Mitigation

0으로 나누려고 하는지 명시적으로 확인하여 핸들링한다.

Memo

알아서 revert 되긴 하는데.. 그래도 명시적으로 처리를 하는 게 나은듯? 지적해볼만 한 듯.


tags: bughunting, smart contract, biconomy, account abstraction, erc4337, division by zero, arithmetic error, wallet, severity low