Digital garden

code4rena-2023-01-biconomy-m02

6 min read

code4rena-2023-01-biconomy-m02

[M-02] Non-compliance with EIP-4337

보고서

Summary

잘못 구현하여 표준에 맞지 않음

Keyword

wrong implementation, standard, bug

Vulnerability

ERC-4337 스펙을 제대로 따르지 않은 구현이 존재함을 지적했다.

Sender existence

ERC-4337 문서의 Required entry point contract functionality 섹션에서는 올바른 Entry point를 구현하기 위하여 반드시 지켜야하는 기능을 제시한다.

Create the account if it does not yet exist, using the initcode provided in the UserOperation. If the account does not exist, and the initcode is empty, or does not deploy a contract at the “sender” address, the call must fail.

이 요구사항은 EntryPoint에 요청이 들어왔을 시, 트랜잭션을 호출하고자 하는 opInfo.mUserOp.sender의 계정이 아직 존재하지 않는다면 파라미터로 받은 initCode를 이용하여 sender 계정을 생성하라는 의미이다. 계정이 존재하지 않을 때, initcode 파라미터가 없다면 콜이 실패해야 한다. 또한 계정 생성을 시도했지만 파라미터로 받은 opInfo.mUserOp.sender 와 다른 주소에 컨트랙트가 배포되었다면 콜이 실패해야 한다. 이 요구사항을 제대로 지키지 못했다는 점이 문제였다.

function _createSenderIfNeeded(uint256 opIndex, UserOpInfo memory opInfo, bytes calldata initCode) internal {
    if (initCode.length != 0) {
        address sender = opInfo.mUserOp.sender;
            if (sender.code.length != 0) revert FailedOp(opIndex, address(0), "AA10 sender already constructed");
            address sender1 = senderCreator.createSender{gas: opInfo.mUserOp.verificationGasLimit}(initCode);
        if (sender1 == address(0)) revert FailedOp(opIndex, address(0), "AA13 initCode failed or OOG");
        if (sender1 != sender) revert FailedOp(opIndex, address(0), "AA14 initCode must return sender");
        if (sender1.code.length == 0) revert FailedOp(opIndex, address(0), "AA15 initCode must create sender");
        address factory = address(bytes20(initCode[0:20]));
        emit AccountDeployed(opInfo.userOpHash, sender, factory, opInfo.mUserOp.paymaster);
	}
}

이 기능을 구현하는 EntryPoint._createSenderIfNeeded 함수이다. 위 코드에서는 if (initCode.length != 0)를 먼저 확인한 후 if (sender.code.length != 0) 를 확인한다. 만약 Sender가 존재하지 않지만 initCode를 넘기지 않았다면 어떻게 되어야 하는가? 스펙을 따르려면 계정이 존재하지 않을 때, initcode 파라미터가 없다면 콜이 실패해야 한다. 하지만 조건문의 순서로 인해 이를 따르지 않는다.

다음과 같이 확인 순서를 바꾸어 Sender가 존재하지 않을 때, initCode가 주어지지 않은 경우를 명시적으로 revert 처리하도록 수정해야 한다.

function _createSenderIfNeeded(uint256 opIndex, UserOpInfo memory opInfo, bytes calldata initCode) internal {
    address sender = opInfo.mUserOp.sender;
    if (sender.code.length == 0) {
        require(initCode.length != 0, "empty initcode");
        address sender1 = senderCreator.createSender{gas: opInfo.mUserOp.verificationGasLimit}(initCode);
        if (sender1 == address(0)) revert FailedOp(opIndex, address(0), "AA13 initCode failed or OOG");
        if (sender1 != sender) revert FailedOp(opIndex, address(0), "AA14 initCode must return sender");
        if (sender1.code.length == 0) revert FailedOp(opIndex, address(0), "AA15 initCode must create sender");
        address factory = address(bytes20(initCode[0:20]));
        emit AccountDeployed(opInfo.userOpHash, sender, factory, opInfo.mUserOp.paymaster);
	}
}

출처: https://github.com/code-423n4/2023-01-biconomy-findings/issues/498

Account

Account를 구현할 시, validateUserOp 함수를 구현해야 한다. function validateUserOp(UserOperation calldatauserOp, bytes32userOpHash, addressaggregator, uint256missingAccountFunds) 이 함수에서는 서명이 올바른지 확인한다. ERC-4337의 스펙 문서에서 예외 상황에 어떻게 처리해야 하는지 명시한다.

If the account does not support signature aggregation, it MUST validate the signature is a valid signature of the userOpHash, and SHOULD return SIG_VALIDATION_FAILED (and not revert) on signature mismatch. Any other error should revert.

Account의 Signature를 검증하고, 만약 올바르지 않다면 트랜잭션을 revert 하지 않고 SIG_VALIDATION_FAILED 를 리턴해야 한다. 하지만 실제 구현에서는 revert 하도록 구현되어 있다.

function _validateSignature(UserOperation calldata userOp, bytes32 userOpHash, address)
    internal override virtual returns (uint256 deadline) {
    bytes32 hash = userOpHash.toEthSignedMessageHash();
    // ignore signature mismatch of from==ZERO_ADDRESS (for eth_callUserOp validation purposes)
    require(owner == hash.recover(userOp.signature) || tx.origin == address(0), "account: wrong signature");
    return 0;
}

또한 validateUserOp 는 time range를 리턴하는게 스펙인데 반해, 실제 구현은 deadline을 리턴한다.

The return value MUST be packed of authorizer, validUntil and validAfter timestamps.

  • authorizer - 0 for valid signature, 1 to mark signature failure. Otherwise, an address of an authorizer contract. This ERC defines “signature aggregator” as authorizer.
  • validUntil is 6-byte timestamp value, or zero for “infinite”. The UserOp is valid only up to this time.
  • validAfter is 6-byte timestamp. The UserOp is valid only after this time
function validateUserOp(UserOperation calldata userOp, bytes32 userOpHash, address aggregator, uint256 missingAccountFunds)
external override virtual returns (uint256 deadline) {
    _requireFromEntryPoint();
    deadline = _validateSignature(userOp, userOpHash, aggregator);
    if (userOp.initCode.length == 0) {
        _validateAndUpdateNonce(userOp);
    }
    _payPrefund(missingAccountFunds);
}

Impact

표준을 정확히 지키지 않아 다른 시스템과 연동할 시 실제 의도대로 동작하지 않을 수 있다.

Mitigation

표준에서 요구하는 기능을 정확히 구현하도록 수정한다.

Memo

취약점이라기보다는 기능 자체의 구현을 잘못한 것이다. 자칫 이 정도는 당연히 지켰겠지 라거나, 좀 잘못되어 보이지만 딱히 취약점은 아니니 지나칠 수도 있겠다. 바운티를 여는 시점에도 이런 수준의 오류가 충분히 있을 수 있다고 생각하고 이상해보이는 것은 일단 지적해보자.

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

Graph View