Digital garden

code4rena-2024-10-kleidi-m03

5 min read

code4rena-2024-10-kleidi-m03

[M-03] UpdateExpirattionPeriod() cannot be executed when the newExpirationPeriod is less than currentExpirationPeriod

보고서

Summary

만료 기간을 업데이트하는 관리자 함수를 실행한 뒤에 다시 만료 여부를 확인한다. 만료 기간을 짧게 수정했다면 다시 확인할 때 조건을 만족하지 못하여 트랜잭션이 취소될 수 있다.

Keyword

logic flaw, timelock

Vulnerability

타임록에 등록된 제안이 실행되면 실행된 제안으로 표시하기 위해 실행 시점의 타임스탬프를 저장한다. 그런데 이를 저장하기 전 먼저 isOperationReady 를 통해 이 id가 이미 실행처리 되었는지를 확인한다. execute 에서 실행하기 전에도 확인했지만 한번 더 확인한다.

isOperationReady 에서는 해당 제안이 이미 실행되었는지, 취소 또는 등록된 적이 없는지, 그리고 expirationPeriod 가 지나지 않았는지를 확인한다. 제안이 등록된 후 너무 오랫동안 실행되지 않은 제안은 만료된 것으로 간주하여 실행할 수 없도록 한다.

    function execute(
        address target,
        uint256 value,
        bytes calldata payload,
        bytes32 salt
    ) external payable whenNotPaused {
        bytes32 id = hashOperation(target, value, payload, salt);
 
        /// first reentrancy check, impossible to reenter and execute the same
        /// proposal twice
        require(_liveProposals.remove(id), "Timelock: proposal does not exist");
@>      require(isOperationReady(id), "Timelock: operation is not ready");
 
        _execute(target, value, payload);
        emit CallExecuted(id, 0, target, value, payload);
 
        /// second reentrancy check, second check that operation is ready,
        /// operation will be not ready if already executed as timestamp will
        /// be set to 1
@>      _afterCall(id);
    }
 
    function _afterCall(bytes32 id) private {
        /// unreachable state because removing the proposal id from the
        /// _liveProposals set prevents this function from being called on the
        /// same id twice
@>      require(isOperationReady(id), "Timelock: operation is not ready");
@>      timestamps[id] = _DONE_TIMESTAMP;
    }
 
    function isOperationReady(bytes32 id) public view returns (bool) {
        /// cache timestamp, save up to 2 extra SLOADs
        uint256 timestamp = timestamps[id];
@>      return timestamp > _DONE_TIMESTAMP && timestamp <= block.timestamp
@>          && timestamp + expirationPeriod > block.timestamp;
    }

타임록 관리 함수 중에는 updateExpirationPeriod 가 있다. 이 함수는 expirationPeriod 시간을 조정할 수 있는 함수이다. 최소 딜레이인 하루 이상으로 등록할 수 있다. 이 관리 함수는 반드시 타임록에 제안을 등록하여, 타임록을 통해 호출되어야 한다.

@>  function updateExpirationPeriod(uint256 newPeriod) external onlyTimelock {
        require(newPeriod >= MIN_DELAY, "Timelock: delay out of bounds");
 
        emit ExpirationPeriodChange(expirationPeriod, newPeriod);
        expirationPeriod = newPeriod;
    }

즉, execute 를 통해 등록해둔 제안을 실행, updateExpirationPeriod 를 호출하여 expirationPeriod 를 조정한 뒤 _afterCall 을 호출하면 이번에 업데이트 된 expirationPeriod 를 기준으로 만료 여부를 확인한다. timestamp + expirationPeriod > block.timestamp 에서 true를 리턴해야 트랜잭션이 정상적으로 종료되는데 새로운 expirationPeriod가 작아진다면 트랜잭션이 취소될 수 있다.

다음 상황을 생각해보자.

  • updateExpirationPeriod 로 expirationPeriod 을 5일로 변경 요청
  • 등록 시점: n
  • 실행 딜레이: 3일
  • 실행 가능 시점(timestamps[id]): n + 3일
  • expirationPeriod: 10일
  • 만료 시점(timestamp + expirationPeriod): n + 3일 + 10일
  • n + 9일 시점에 execute 실행
    • 아직 만료되지 않았으므로 실행 가능
    • 실행 마지막에 _afterCall 호출
      • expirationPeriod: 5일
      • 이번 제안 만료 시점(timestamp + expirationPeriod): n + 3일 + 5일
        • 업데이트된 expirationPeriod로는 만료시점이 지나므로 트랜잭션 취소됨

Impact

만료 기간을 줄이는 제안을 실행할 수 없다.

Mitigation

_afterCall 에서는 isOperationReady 를 다시 확인하지 않는다.

tags: bughunting, kleidi, smart contract, solidity, gnosis safe, wallet, logic flaw, severity medium

Graph View