code4rena-2023-01-biconomy-h01

[H-01] Destruction of the SmartAccount implementation

보고서

Summary

Proxy의 Implementation 컨트랙트를 selfdestuct 할 수 있다. 이로써 전체 시스템을 중단시킬 수 있다.

Keyword

delegateCall, proxy, signature, ecrecover, authentication, deploy, initialize, selfdestuct

Vulnerability

SmartAccount 컨트랙트는 Proxy 패턴을 이용한다. 그런데, SmartAccount 에 대한 Implementation 컨트랙트를 초기화하지 않아서 문제가 발생했다. 정확히 말하자면 owner 값을 초기화하지 않았기 때문이다.

const SmartWallet = await ethers.getContractFactory("SmartAccount");
const baseImpl = await SmartWallet.deploy();
await baseImpl.deployed();
console.log("base wallet impl deployed at: ", baseImpl.address);

이것이 어떻게 취약점을 야기하는가? 다음과 같은 조건이 성립하여 공격이 가능하게 한다.

  1. ecrecover 는 잘못된 input을 받은 경우 address(0) 를 리턴한다.
  2. owner가 초기화되지 않아 디폴트 값 address(0) 이다.
  3. SmartAccount는 EOA 대용으로, 임의의 delegateCall을 실행하는 기능이 존재한다.

SmartAccount는 간략하게, 서명으로 호출자 owner 확인 실행 작업을 수행한다. 잘못된 서명을 넣어 address(0)가 나오면 owner 확인을 우회할 수 있다.

이를 통해 악성 컨트랙트에 delegateCall을 호출하면 어떻게 되는가?

contract Destructor {
    fallback() external {
        selfdestruct(payable(0));
    }
}

다음과 같이 selfdestruct 하는 컨트랙트에 delegateCall을 하면 SmartAccount Implementation 컨트랙트가 삭제된다. 로직 컨트랙트가 삭제되었기 때문에 Proxy들의 Implementation을 변경할 수도 없다. updateImplementation 를 호출해야 하는데 이 호출할 로직이 삭제되었기 때문이다.

Impact

Implementation 컨트랙트가 삭제되었으므로 연결된 모든 SmartAccount (Proxy)의 기능이 동결된다. 서비스 종료 버튼인 것이다.

Mitigation

Implementation의 Deploy 스크립트에서 init() 함수를 호출한다. 또는 Construtor에서 owneraddress(0) 가 아닌 값으로 설정하여 초기화한다.

// Constructor ensures that this implementation contract can not be initialized
constructor() public {
    owner = address(1);
}

Memo

굉장히 크리티컬한 버그이다. 많은 사람이 제출한 것으로 보아 취약점의 유형 자체는 꽤 흔한듯 하다.

기존에는 _disableInitializers() 를 호출하는 것 만으로 충분한가 생각했는데, 이번케이스는 Initalizeable도 아닐 뿐더러, 설령 _disableInitializers() 처리를 했더라 하더라도 owner 디폴트 값에 의한 우회는 피할 수 없었을 것이다.

  constructor() {
        _disableInitializers();
    }

Proxy 패턴이 사용되었다면 초기화와 관련된 이슈가 없는지 접근해보자.

또한 애초에 delegateCall이 가능하도록 하는게 맞는지 의문이다. SmartAccount 컨트랙트로 delegateCall을 시켰을 때 이점이 뭔가?


tags: bughunting, smart contract, biconomy, account abstraction, erc4337, delegateCall, proxy pattern, signature, ecrecover, authentication-vul, selfdestruct, initialize error, wallet, severity high