This article belongs to a series of posts intended to share some nerdy insights from our latest spotcheck on the implementation of the imposing ERC-4337 for Account Abstraction.

This is the second article of the series. If you haven't read it, here's the first one:

Pointers in Solidity?
Learning about memory pointers in Solidity using assembly.

Error handling in Solidity

If you didn't know that you can handle errors in Solidity, well, now you know!

At the very least, start by skimming through the first paragraphs of Error handling: Assert, Require, Revert and Exceptions, and maybe check out the try/catch code from Solidity by example.

The source of my curiosity

While peeking at the validations inside the EntryPoint contract, I stumbled upon the following piece of code inside the _validatePaymasterPrepayment function.

try IPaymaster(paymaster).validatePaymasterUserOp(op ...)
    returns (bytes memory, uint256) {
    ...
} catch Error(string memory revertReason) {
    revert FailedOp(..);
} catch {
    revert FailedOp(..);
}
Simplified version of the _validatePaymasterPrepayment function

Look at the external call to the validatePaymasterUserOp function.

This made me wonder... what if a paymaster (an actor that can sponsor user operations) does some nasty stuff when its handled execution control ? Could it somehow force a revert in the context of the EntryPoint contract without emiting a FailedOp()? How would that affect the rest of the code?

I knew there was a reputation system and what not, but still I decided to chase this idea to see where it could lead. Curiosity-driven articles!

My experiments

Here's a simple scenario to experiment with Solidity's try/catch. I'm going to use sample code from Solidity's official documentation with minor tweaks. It's about a contract that tries to call a function from another contract, and catches the error if it fails.

If you want to go ahead and check the entire code, get your hands dirty with this Foundry repository I created.

Let's begin by defining an interface (I'm calling it DataFeed) for contracts whose only purpose is to provide data through the function getData. Data will be in the form of a uint256 value.

interface DataFeed { 
	function getData() external returns (uint256 value);
}
DataFeed interface

A simple implementation of such interface could be:

contract DataFeedOK is DataFeed {
    function getData() public pure returns (uint256 value) {
        return 1;
    }
}
A common DataFeed type contractβ€Œ

Now, the data is going to be retrieved by a data feed gatherer that delegates the retrieval to the given contract using a function I'm naming getDataFromFeed(DataFeed feed).

contract FeedDataGatherer {
    function getDataFromFeed(DataFeed feed) public returns (uint value, bool success) {
        try feed.getData() returns (uint v) {
            return (v, true);
        } catch Error(string memory /*reason*/) {
            // This is executed in case
            // revert was called inside getData
            // and a reason string was provided.
            return (0, false);
        } catch Panic(uint /*errorCode*/) {
            // This is executed in case of a panic,
            // i.e. a serious error like division by zero
            // or overflow. The error code can be used
            // to determine the kind of error.
            return (0, false);
        } catch (bytes memory /*lowLevelData*/) {
            // This is executed in case revert() was used.
            return (0, false);
        }
    }
}
FeedDataGatherer contract

As you can see, the previous contract seems to cover everything needed for a robust error handling.

Or does it? πŸ€”

What if you found an error-handling piece of code of this sort where a failure to catch a revert would lead to a denial of service attack ? Do you think it'd be possible to reach a revert bypassing all implemented catch clauses ?

From now on, I'll try to use my adversarial mindset to do exactly that, and see where we land.

πŸ—’οΈ
For the following examples I'll be calling getDataFromFeed() from a deployed FeedDataGatherer, passing as parameter the address of each evil deployed contract.

I'll skip creating evil data feed contracts using custom errors because that's something we will get caught for sure, so let's start with something else.

Case 1: Returning values of different type

What if the receiver of the external call returned a signed integer, instead of an unsigned integer ?

contract EvilFeedContractA {
    function getData() public pure returns (int value) {
        return(-1);
    }
}
Evil A - returns another numerical type

No reverts. Here's the output I saw in Remix:
value: 1157920892373161954235709....564039457584007913129639935β€Œβ€Œ
success: true

Wait! And what if the receiver returned another type of a lower size? Say for example, an address instead of a uint256.

contract EvilFeedContractB {
    function getData() public pure returns (address addr) {
        
        return(address(0x28C565e19D87a092Ab89aa8Fe7601ffbF8878AFf));
    }
}
Evil B - returns a type of lower size

Again, no reverts. The output I got in Remix:
value: 232761752750868790944344549983973884987338885887β€Œβ€Œ
success: true

In these first two examples, Solidity is doing an implicit type casting or interpretation of the returned value. It is saying: this I'm getting right here should be a uint256 according to the interface set, so I'm gonna read it like that.

With that in mind, let's make sense of the values we got so far.

For the first example, the equivalent to a int256 of -1 interpreted as a uint256 is the value (2^256 - 1). You can verify this with Chisel like this:

matt at webtrES in ~/theredguild 
β†ͺ chisel
➜ int(-1)
Type: int β”œ Hex: 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff Decimal: -1 
Foundry's chisel utility

For the case of the address, whose value is 0x28C565e19D87a092Ab89aa8Fe7601ffbF8878AFf, is just that hex value as an unsigned integer. You can also verify this:

matt at webtrES in ~/theredguild
β†ͺ bpython
>>> 0x28C565e19D87a092Ab89aa8Fe7601ffbF8878AFf
232761752750868790944344549983973884987338885887

But still, no reverts! 😀

Case 2: returning values from a different location

Let's change the location of the return variable to see if we can disrupt it a little, for example, with memory.

Here's with a string memory parameter type.

contract EvilFeedContractC {
    function getData() public pure returns (string memory value) {
        
        return("Vamo' lo pibeeeeeeeeeeeeeeee");
    }
}
Evil C - returns a different type and location

The output in Remix:
value: 32β€Œβ€Œ
success: true

🎈
Can you guess why is it returning 32? I'll leave this as an exercise for you.

For some reason it's still getting something it can work with, so no reverts.

Let's move on, and try with a dynamic array:

contract EvilFeedContractD {
    function getData() public pure returns (uint256[] memory) {
        uint256[] memory a;
        a[0] = 2;
        a[1] = 1;
        a[2] = 0;

        return a;
    }
}
Evil D - dynamic structure

The output in Remix:
value: 0β€Œβ€Œ
success: false

If you were to debug this, you will find that it is reverting with the string "Index out of bounds" as a reason, triggering the first catch, and as a consequence executing return(0, false), hence the output.

  [11270] CatchMeIfYouCanTest::test_evilD() 
    β”œβ”€ [3507] FeedDataGatherer::getDataFromFeed(EvilFeedContractD: [0xc7183455a4C133Ae270771860664b6B7ec320bB1]) 
    β”‚   β”œβ”€ [201] EvilFeedContractD::getData() 
    β”‚   β”‚   └─ ← "Index out of bounds"
    β”‚   └─ ← 0, false
    └─ ← ()
Foundry trace for evil D's test
Case 3: not returning data at all

I was running out of ideas! So I decided to do something more drastic. What if I were to remove the return data altogether? Because the returned data is not part of the function identifier, the function is still called.

Here's the contract:

contract EvilFeedContractE {
    uint256 count;
    function getData() public {
        count += 1;
    }
}
Evil E - counter increment

The output in Remix:
Execution failed. Failed to decode output.
Error: hex data is odd-length(argument=\"value\", value=\"0x0\", code=INVALID_ARGUMENT, version=bytes/5.5.0)"

Yay! I managed to trigger an uncaught revert. But, why?

  [33088] CatchMeIfYouCanTest::test_evilE() 
    β”œβ”€ [25400] FeedDataGatherer::getDataFromFeed(EvilFeedContractE: [0xa0Cb889707d426A7A386870A03bc70d1b0697598]) 
    β”‚   β”œβ”€ [22322] EvilFeedContractE::getData() 
    β”‚   β”‚   └─ ← ()
    β”‚   └─ ← "EvmError: Revert"
    └─ ← "EvmError: Revert"
Foundry trace for evil E's trace

The try I coded in FeedDataGatherer expects the external call to return a value (a uint256), but doesn't know how to handle situations where there isn't anything to be returned!

It seems expected behavior, doesn't it? Let's see if we can simplify this case further. What would be the shortest piece of code for a contract to execute a function with no content?

A contract with an empty fallback!

contract EvilFeedContractF {
    fallback() external {
        // do nothing!
    }
}
Evil F - do nothing!

Unsurprisingly, when executed, the output yields the same result as the execution of EvilFeedContractE.

There are other ways to bypass catch statements and trigger unhandled reverts. For example, you could pass an address where there is no code deployed at all. But in my experience that'd be a more rare scenario, because of codesize checks usually implemented as defense mechanisms.

What did we learn?

In Solidity, try/catch statements are used to handle failures in external calls. When you make an external function call with a try block, you specify one or more catch blocks to handle potential errors.

In the case of a failure, the catch block is only triggered if the external call fails in a way that Solidity can recognize and handle – such as when the called function runs out of gas, doesn't exist, or something like an under/overflow appears.

But there are edge cases that developers might not be aware of.

Like when the output cannot be decoded to the expected types, or the called account doesn't have any code.

In words of the Solidity documentation:

If an error happens during the decoding of the return data inside a try/catch-statement, this causes an exception
in the currently executing contract and because of that, it is not caught in the catch clause.

In our spotcheck, things ended up being just fine. But as security reviewers, we should always be thinking about these cases.

Can I assume you're ready to try and catch our next adventure ?