codehawks-2023-08-sparkn-m01

[M-01] The digest calculation in deployProxyAndDistributeBySignature does not follow EIP-712 specification

보고서

Summary

서명되는 데이터가 EIP712 사양을 정확히 지키지 않았다. EIP712 사양을 지킨 지갑을 이용했을 때 실패할 것이다.

Keyword

wrong implementation, erc712, signature

Vulnerability

ProxyFactory 컨트랙트에서 서명을 확인하기 위해 오픈제플린 EIP712 컨트랙트의 _hashTypedDataV4 함수로 해시된 데이터를 EIP712 포맷에 맞추려 한다. 이 때, 파라미터로 abi.encode(contestId, data)에 대한 해시를 넣었다.

    function deployProxyAndDistributeBySignature(
        address organizer,
        bytes32 contestId,
        address implementation,
        bytes calldata signature,
        bytes calldata data
    ) public returns (address) {
        bytes32 digest = _hashTypedDataV4(
            keccak256(
                abi.encode(contestId, data)
            )
        );
        ...
    }

EIP-712 specification 문서에서는 메시지를 "\x19\x01" ‖ domainSeparator ‖ hashStruct(message) 와 같이 인코딩하라고 명시한다.

앞의 \x19\x01 prefix와 domainSeparator는 _hashTypedDataV4에서 붙여주므로 _hashTypedDataV4 함수의 파라미터로는 hashStruct(message)만 넣으면 된다. 그런데 abi.encode(contestId, data)hashStruct(message)의 조건을 정확히 충족하지 못한다.

  1. 메시지에 typeHash가 포함되지 않음
  2. bytes와 string과 같은 동적인 데이터는 실제 값 대신 해시를 이용해야 함

메시지에 typeHash가 포함되지 않음

hashStructhashStruct(s : 𝕊) = keccak256(typeHash ‖ encodeData(s))로 데이터 앞에 typeHash가 붙어야 한다. 이 typeHashtypeHash = keccak256(encodeType(typeOf(s))) 이다. 즉, 서명한 데이터들에 대한 정보가 포함되어야 한다. typeHash는 컴파일타임에 미리 계산해두고 이용한다. 예를 들어 다음과 같이 정의한다.

    bytes32 constant MAIL_TYPEHASH = keccak256("Mail(address from,address to,string contents)");

bytes와 string과 같은 동적인 데이터는 실제 값 대신 해시를 이용해야 함

EIP712 사양에 의하면 encodeData에 들어가는 bytes, string과 같은 동적인 자료형은 실제 값 대신 이들의 keccak256 해시를 해야 한다고 명시한다.

Impact

서명되는 데이터가 EIP712 사양에 따라 인코딩되지 않아, 적절한 방식으로 인코딩을 수행하는 EIP712 호환 지갑 또는 도구가 예기치 않게 실패할 수 있다.

Mitigation

먼저 typeHash를 정의한다. 데이터에 적당한 이름을 붙이고 인코딩한 데이터의 순서대로 나열, 타입을 명시한 문자열을 해시한다.

    bytes32 internal constant DEPLOY_AND_DISTRIBUTE_TYPEHASH = keccak256(
        "DeployAndDistribute(bytes32 contestId,bytes data)"
    );

그리고 digest를 생성할 때 파라미터에 typeHash를 넣고, data는 keccak256 하여 사용한다.

    function deployProxyAndDistributeBySignature(
        address organizer,
        bytes32 contestId,
        address implementation,
        bytes calldata signature,
        bytes calldata data
    ) public returns (address) {
        bytes32 digest = _hashTypedDataV4(
            keccak256(
-                abi.encode(contestId, data)
+                abi.encode(
+                   DEPLOY_AND_DISTRIBUTE_TYPEHASH,
+                   contestId,
+                   keccak256(data)
+               )
            )
        );
        ...
    }

Memo

EIP712를 정확히 지키지 않아 지갑과 호환되지 않을 수 있다는 점을 이유로 든 것이 합리적이다.


tags: bughunting, sparkn, smart contract, solidity, erc712, signature, severity medium