Result of transfer not checked

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L602 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L605

Vulnerability details

Impact

Several calls to transfer are done without checking the results. For certain ERC20 tokens, if insufficient tokens are present, no revert occurs but a result of "false" is returned. So it's important and also a best practice to check this.

Proof of Concept

Use of transfer without result-checking:

contracts/RubiconRouter.sol:
  251:             ERC20(route[route.length - 1]).transfer(to, currentAmount);
  303:         ERC20(buy_gem).transfer(msg.sender, fill);
  320:         ERC20(buy_gem).transfer(msg.sender, fill);
  348:         ERC20(buy_gem).transfer(msg.sender, buy_amt);
  377:             ERC20(pay_gem).transfer(msg.sender, max_fill_amount - fill);
  406:             ERC20(buy_gem).transfer(msg.sender, _after - _before);
  471:         ERC20(targetPool).transfer(msg.sender, newShares);

contracts/peripheral_contracts/BathBuddy.sol:
  114:             token.transfer(recipient, amountWithdrawn);

contracts/rubiconPools/BathPair.sol:
  601:             IERC20(asset).transfer(msg.sender, booty);
  615:             IERC20(quote).transfer(msg.sender, booty);

contracts/rubiconPools/BathToken.sol:
  353:         IERC20(filledAssetToRebalance).transfer(
  357:         IERC20(filledAssetToRebalance).transfer(msg.sender, stratReward);
  602:             underlyingToken.transfer(feeTo, _fee);
  605:         underlyingToken.transfer(receiver, amountWithdrawn);

Recommended Mitigation Steps

Always check the result of transfer or use safeTransfer.

Note that safeTransfer is used here:

contracts/peripheral_contracts/VestingWallet.sol:
  109:         SafeERC20.safeTransfer(IERC20(token), beneficiary(), releasable);

Note that results of transfer are checked here:

contracts/RubiconMarket.sol:
  297          require(
  298:             _offer.buy_gem.transferFrom(msg.sender, feeTo, fee),
  299              "Insufficient funds to cover fee"

  304          require(
  305:             _offer.buy_gem.transferFrom(msg.sender, _offer.owner, spend),
  306:             "_offer.buy_gem.transferFrom(msg.sender, _offer.owner, spend) failed - check that you can pay the fee"
  307          );
  308          require(
  309:             _offer.pay_gem.transfer(msg.sender, quantity),
  310:             "_offer.pay_gem.transfer(msg.sender, quantity) failed"
  311          );

  361:         require(_offer.pay_gem.transfer(_offer.owner, _offer.pay_amt));

  416:         require(pay_gem.transferFrom(msg.sender, address(this), pay_amt));

[KenzoAgada]

Duplicate of #316.

[bghughes]

Duplicate of #316