codehawks-2023-08-sparkn-l02

[L-02] Owner can incorrectly pull funds from contests not yet expired

보고서

Summary

proxy 주소가 실제로 salt를 이용해 배포된 주소인지 확인하지 않아 시간 제한을 우회하여 proxy의 함수를 호출할 수 있다.

Keyword

lack of input validation

Vulnerability

distributeByOwner 함수는 정산 후 실수로 토큰을 보낸 이가 있다면, EXPIRATION_TIME(7일)이 지난 후 관리자가 구출해주는 기능이다.

그러나 관리하고자 하는 proxy와 주소가 실제로 salt를 사용해 배포된 컨트랙트인지 확인하지 않는다. EXPIRATION_TIMEsalt를 기준으로 조회하기 때문에, saltEXPIRATION_TIME가 종료된 컨테스트로 설정하면 아직 EXPIRATION_TIME가 지나지 않은 컨테스트 proxydistributeByOwner를 호출해 토큰을 빼낼 수 있다.

    function distributeByOwner(
        address proxy,
        address organizer,
        bytes32 contestId,
        address implementation,
        bytes calldata data
    ) public onlyOwner {
        if (proxy == address(0)) revert ProxyFactory__ProxyAddressCannotBeZero();
        bytes32 salt = _calculateSalt(organizer, contestId, implementation);
        if (saltToCloseTime[salt] == 0) revert ProxyFactory__ContestIsNotRegistered();
        // distribute only when it exists and expired
        if (saltToCloseTime[salt] + EXPIRATION_TIME > block.timestamp) revert ProxyFactory__ContestIsNotExpired();
        _distribute(proxy, data);
    }
 
    function _distribute(address proxy, bytes calldata data) internal {
        (bool success,) = proxy.call(data);
        if (!success) revert ProxyFactory__DelegateCallFailed();
        emit Distributed(proxy, data);
    }

Impact

시간 제한을 우회하여 distributeByOwner 함수로 관리자가 토큰을 꺼낼 수 있다.

Mitigation

proxy 주소가 salt를 사용하여 배포된 게 맞는지 확인한다.

function distributeByOwner(
    address proxy,
    address organizer,
    bytes32 contestId,
    address implementation,
    bytes calldata data
) public onlyOwner {
    if (proxy == address(0)) revert ProxyFactory__ProxyAddressCannotBeZero();
    bytes32 salt = _calculateSalt(organizer, contestId, implementation);
    if (saltToCloseTime[salt] == 0) revert ProxyFactory__ContestIsNotRegistered();
    // distribute only when it exists and expired
    if (saltToCloseTime[salt] + EXPIRATION_TIME > block.timestamp) revert ProxyFactory__ContestIsNotExpired();
+	require(getProxyAddress(salt, implementation) == proxy);
    _distribute(proxy, data);
}

tags: bughunting, sparkn, smart contract, solidity, lack-of-input-validation-vul, severity low