code4rena-2023-01-biconomy-m06

[M-06] Doesn’t Follow ERC1271 Standard

보고서

Summary

Magic value를 임의로 변경하여 외부 컨트랙트에서 서명을 인증하는 기능이 항상 실패한다.

Keyword

erc 1271, signature, magic value, protocol, bug

Vulnerability

ERC-1271: Standard Signature Validation Method for Contracts 를 정확히 따르지않았다. 서명을 검증하는 isValidSignature 함수에서, 서명이 올바를 때 리턴해야 하는 값은 0x1626ba7e 이다. 이는 bytes4(keccak256("isValidSignature(bytes32,bytes)") 로, 일종의 약속된 값이라고 보면 된다.

 contract ERC1271 {
 
  // bytes4(keccak256("isValidSignature(bytes32,bytes)")
  bytes4 constant internal MAGICVALUE = 0x1626ba7e;
 
  /**
   * @dev Should return whether the signature provided is valid for the provided hash
   * @param _hash      Hash of the data to be signed
   * @param _signature Signature byte array associated with _hash
   *
   * MUST return the bytes4 magic value 0x1626ba7e when function passes.
   * MUST NOT modify state (using STATICCALL for solc < 0.5, view modifier for solc > 0.5)
   * MUST allow external calls
   */ 
  function isValidSignature(
    bytes32 _hash, 
    bytes memory _signature)
    public
    view 
    returns (bytes4 magicValue);
}

하지만 실제 구현에서는 이 Magic value를 0x20c13b0b 로 잘못 지정하였다.

contract ISignatureValidatorConstants {
    // bytes4(keccak256("isValidSignature(bytes,bytes)")
    bytes4 internal constant EIP1271_MAGIC_VALUE = 0x20c13b0b;
}

이 값은 SmartAccount.checkSignatures 함수에서 외부 컨트랙트에게 서명의 해석을 맡길 때 이용된다. 올바르게 ERC-1271을 따르는 외부 컨트랙트에서는 서명 인증에 성공한 경우 0x1626ba7e 를 리턴할 것이다. 하지만 SmartAccount에서는 이를 잘못된 Magic value와 비교하여 서명이 틀렸다고 처리한다.

function checkSignatures(
    bytes32 dataHash,
    bytes memory data,
    bytes memory signatures
) public view virtual {
    uint8 v;
    bytes32 r;
    bytes32 s;
    uint256 i = 0;
    address _signer;
    (v, r, s) = signatureSplit(signatures, i);
    //review
    if(v == 0) {
        ...
        require(ISignatureValidator(_signer).isValidSignature(data, contractSignature) == EIP1271_MAGIC_VALUE, "BSA024");
    }
    ...
}

Impact

외부 서명 인증이 항상 실패하여 기능하지 않는다.

Mitigation

Magic value를 ERC-1271를 따르도록 올바르게 수정한다.


tags: bughunting, smart contract, biconomy, account abstraction, erc4337, erc1271, signature, wallet, severity medium