Digital garden

code4rena-2021-06-pooltogether-m04

4 min read

code4rena-2021-06-pooltogether-m04

[M-04] The assumption that operator == to (user) may not hold leading to failed timelock deposits

보고서

Summary

출금할 때와 입금할 때 msg.sender와 from/to 파라미터가 동일 주소일 것으로 가정했다. 메타 트랜잭션을 이용할 경우 요청자가 msg.sender와 다를 수 있으며, 이 경우 출금시와 입금시의 처리가 불일치하여 정상적으로 처리되지 않는다.

Keyword

msg.sender, meta transaction

Vulnerability

timelockDepositTo 함수는 유저가 기존에 출금 요청을 하여 Timelock이 걸려있는 토큰을 다시 풀에 입금하는 함수이다.

이 함수는 operator, 즉 _msgSender()의 _timelockBalances의 잔액을 업데이트 한다. operator 소유의 Timelock 걸린 토큰 수를 줄이며 파라미터로 받은 to의 명의로 입금한다. (영수증 토큰을 mint)

  function timelockDepositTo(
    address to,
    uint256 amount,
    address controlledToken
  )
    external
    onlyControlledToken(controlledToken)
    canAddLiquidity(amount)
    nonReentrant
  {
    address operator = _msgSender();
    _mint(to, amount, controlledToken, address(0));
    _timelockBalances[operator] = _timelockBalances[operator].sub(amount);
    timelockTotalSupply = timelockTotalSupply.sub(amount);
 
    emit TimelockDeposited(operator, to, controlledToken, amount);
  }

다음은 수수료를 떼이는 대신 Timelock을 걸며 출금 요청을 하는 withdrawWithTimelockFrom 함수이다. 여기서는 파라미터로 받은 from의 Timelock 잔고를 더하며, 영수증 토큰은 _msgSender()의 것을 burn 한다.

  function withdrawWithTimelockFrom(
    address from,
    uint256 amount,
    address controlledToken
  )
    external override
    nonReentrant
    onlyControlledToken(controlledToken)
    returns (uint256)
  {
    uint256 blockTime = _currentTime();
    (uint256 lockDuration, uint256 burnedCredit) = _calculateTimelockDuration(from, controlledToken, amount);
    uint256 unlockTimestamp = blockTime.add(lockDuration);
    _burnCredit(from, controlledToken, burnedCredit);
    ControlledToken(controlledToken).controllerBurnFrom(_msgSender(), from, amount);
    _mintTimelock(from, amount, unlockTimestamp);
    emit TimelockedWithdrawal(_msgSender(), from, controlledToken, amount, unlockTimestamp);
 
    // return the block at which the funds will be available
    return unlockTimestamp;
  }

이 두 함수는 msg.sender와 파라미터로 받은 to 또는 from이 동일 주소일 것을 가정하는 것으로 보인다. 하지만 msg.sender를 직접 사용하는 대신 _msgSender() 함수를 이용해 가져오는 이유는 GSN와 같은 메타 트랜잭션을 이용하는 경우를 처리하기 위해서이다. 이에 대한 충분한 고려가 되지 않았고, 로직의 불일치가 존재한다.

유저가 GSN을 이용한다고 가정해보자. GSN을 이용하는 유저가 withdrawWithTimelockFrom를 호출하면 _mintTimelock(from, amount, unlockTimestamp);에 의해 from, 즉 유저 지갑 주소에 Timelock 잔고가 추가된다. 이후 유저가 Timelock이 걸려있는 출금을 다시 재입금하고 싶어져 timelockDepositTo를 호출한다고 하자. 이 떄 _timelockBalances[operator] = _timelockBalances[operator].sub(amount); 코드로 인해 msg.sender, 즉 relayer의 주소로 발행된 Timelock에서 제하려고 한다. 이 시나리오에서 유저는 자신의 Timelock이 걸린 토큰을 재입금하지 못 할 것이다.

즉, withdrawWithTimelockFrom와 timelockDepositTo의 _timelockBalances 데이터 사용 기준이 다르다. 동일 유저가 동일하게 GSN을 이용했음에도 withdrawWithTimelockFrom에서는 파라미터로 받은 from(유저 주소)에게 Timelock이 발행되고, timelockDepositTo는 _msgSender()(=relayer)의 Timelock 잔고를 삭제한다.

Impact

msg.sender(operator)가 from/to 파라미터 주소와 다른 경우 Timelock된 토큰의 재입금이 실패한다.

Mitigation

withdrawWithTimelockFrom와 동일하게 파라미터로 받은 to의 _timelockBalances를 이용한다.

  function timelockDepositTo(
    address to,
    uint256 amount,
    address controlledToken
  )
    external
    onlyControlledToken(controlledToken)
    canAddLiquidity(amount)
    nonReentrant
  {
    address operator = _msgSender();
    _mint(to, amount, controlledToken, address(0));
-   _timelockBalances[operator] = _timelockBalances[operator].sub(amount);
+   _timelockBalances[to] = _timelockBalances[to].sub(amount);
    timelockTotalSupply = timelockTotalSupply.sub(amount);
 
    emit TimelockDeposited(operator, to, controlledToken, amount);
  }

tags: bughunting, pooltogether, smart contract, solidity, meta transaction, severity medium

Graph View