Digital garden

code4rena-2022-10-blur-exchange-m01

3 min read

code4rena-2022-10-blur-exchange-m01

[M-01] Contract Owner Possesses Too Many Privileges

보고서

Summary

owner에 너무 많은 권한이 있고, 중앙화되어 있다. approve 해둔 토큰을 owner가 쉽게 빼낼 수 있다.

Keyword

rug pull, centralization, ownable, approve

Vulnerability

    function transferERC20(address token, address from, address to, uint256 amount)
        approvedContract
        external
        returns (bool)
    {
        require(revokedApproval[from] == false, "User has revoked approval");
        return IERC20(token).transferFrom(from, to, amount);
    }
 
    modifier approvedContract() {
        require(contracts[msg.sender], "Contract is not approved to make transfers");
        _;
    }
 
    function approveContract(address _contract) onlyOwner external {
        contracts[_contract] = true;
        emit ApproveContract(_contract);
    }

approvedContract로 등록된 주소만 transferERC20 를 호출할 수 있는 구조이다. approveContract는 owner로부터 지정 가능하다.

ExecutionDelegate 컨트랙트는 거래시 ERC20 대금이나 거래된 ERC721, ERC1155를 옮기는 역할을 한다. 유저는 자신의 토큰을 ExecutionDelegate에 approve 해두어야 거래할 수 있다.

owner에게 approveContract 권한이 있으므로, owner가 마음만 먹으면 ExecutionDelegate 컨트랙트에 approve된 모든 토큰을 빼낼 수 있다. 이는 rugpull의 위험이 있고, 실수로 owner의 PK가 유출된 경우에도 리스크가 크다.

revokeApproval를 호출하면 address 단위로 거래를 막을 수 있지만, frontrun을 통해 선수칠 수 있다.

Impact

owner 계정으로 rug pull이 가능하다. rug pull이 아니더라도 owner의 PK가 유출되는 경우 큰 피해가 발생할 수 있다.

Mitigation

프로토콜의 설계 자체가 안전하지 않다. 토큰 transfer 전용 컨트랙트를 따로 두지 말고(approveContract가 필요했던 이유) BlurExchange 컨트랙트에서 바로 토큰을 옮기도록 구조를 변경한다.

또는 governance 를 통해서만 approveContract가 호출되게 하여 owner의 권한을 줄인다.

Memo

이 취약점을 인정해줄까 말까 논쟁이 있었는데, 엔드유저측 손해와 관련되었으며 과거에 admin 권한과 관련된 이슈를 Medium으로 평가한 판례가 있어 Medium을 주었다. 이게 뭐 별건가 싶은데, High과 Medium이 총 2개였어서 그런지 이 취약점을 제보한 사람들은 약 2000달러를 더 받았다.. 거절된다고 손해볼 것도 아니니 소신을 갖고 지적하는게 좋겠다..

tags: bughunting, blur exchange, smart contract, solidity, rug pull, ownable, centralization, severity medium

Graph View

Backlinks