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)의 조건을 정확히 충족하지 못한다.
- 메시지에
typeHash가 포함되지 않음 - bytes와 string과 같은 동적인 데이터는 실제 값 대신 해시를 이용해야 함
메시지에 typeHash가 포함되지 않음
hashStruct는 hashStruct(s : 𝕊) = keccak256(typeHash ‖ encodeData(s))로 데이터 앞에 typeHash가 붙어야 한다. 이 typeHash는 typeHash = 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