Digital garden

code4rena-2021-06-pooltogether-m02

4 min read

code4rena-2021-06-pooltogether-m02

[M-02] Return values of ERC20 transfer and transferFrom are unchecked

보고서

Summary

safeTransfer를 사용하지 않아 ERC20 비호환 토큰 이동에 실패해도 revert 되지 않고 정상처리 된다.

Keyword

safeTransfer, erc20

Vulnerability

ERC20과 완전히 호환되지 않는 토큰의 경우 transfer와 transferFrom의 리턴값을 보고 전송 성공 여부를 확인해야 한다. 토큰 양이 부족해도 revert 되지 않고 단순히 false를 리턴하는 토큰 컨트랙트도 있기 때문이다. 이를 위해 SafeERC20 라이브러리를 이용하기도 한다.

BadgerYieldSource 컨트랙트에서 사용하는 BADGER 토큰은 완벽하게 ERC20으로 호환되지 않는 토큰이다. Etherscan에서 이 컨트랙트의 코드를 보면 토큰 이동 시 잔액이 부족하면 revert 되지 않고, false를 리턴하는 것을 볼 수 있다. 따라서 이 토큰은 리턴값을 확인해 전송 결과를 확인해야 한다.

  function transfer(address _to, uint256 _amount) public returns (bool success) {
    require(transfersEnabled);
    return doTransfer(msg.sender, _to, _amount);
  }
 
  function transferFrom(address _from, address _to, uint256 _amount) public returns (bool success) {
    if (msg.sender != controller) {
        require(transfersEnabled);
 
        // The standard ERC 20 transferFrom functionality
        if (allowed[_from][msg.sender] < _amount)
            return false;
        allowed[_from][msg.sender] -= _amount;
    }
    return doTransfer(_from, _to, _amount);
  }
 
  function doTransfer(address _from, address _to, uint _amount) internal returns(bool) {
    if (_amount == 0) {
      return true;
    }
    require(parentSnapShotBlock < block.number);
    // Do not allow transfer to 0x0 or the token contract itself
    require((_to != 0) && (_to != address(this)));
    // If the amount being transfered is more than the balance of the
    //  account the transfer returns false
    var previousBalanceFrom = balanceOfAt(_from, block.number);
    if (previousBalanceFrom < _amount) {
      return false;
    }
    ...
  }

BadgerYieldSource 컨트랙트에서 다음과 같이 transferFrom와 transfer의 리턴값을 확인하지 않는다. 따라서 실제로 Badger 토큰이 이동하지 않더라도 정상 처리 된다.

  function supplyTokenTo(uint256 amount, address to) public override {
    badger.transferFrom(msg.sender, address(this), amount);
    ...
  }
 
  function redeemToken(uint256 amount) public override returns (uint256) {
    ...
    badger.transfer(msg.sender, badgerBalanceDiff);
    return (badgerBalanceDiff);
  }

SushiYieldSource 컨트랙트에서 다음과 같이 transferFrom와 transfer의 리턴값을 확인하지 않으므로 호환되지 않는 토큰을 이용할 시 동일한 문제가 발생할 것이다.

  function supplyTokenTo(uint256 amount, address to) public override {
    sushiAddr.transferFrom(msg.sender, address(this), amount);
    ...
  }
 
  function redeemToken(uint256 amount) public override returns (uint256) {
    ...
    sushi.transfer(msg.sender, sushiBalanceDiff);
    return (sushiBalanceDiff);
  }

Impact

ERC20 비호환 토큰 이동에 실패해도 revert 되지 않고 정상처리 된다.

Mitigation

Openzeppelin SafeERC20 라이브러리의 safeTransfer와 safeTransferFrom 함수를 사용하여 토큰 이동시 리턴값을 확인하고 실패시 revert 한다.

tags: bughunting, pooltogether, smart contract, solidity, safeTransfer, erc20, severity medium

Graph View