in your storage

Solidity Inline Assembly Vulnerabilities

Using assembly to save gas can introduce memory corruption and other subtle vulnerabilities..

Dacian's photo
Dacian
·

22 min read

Table of contents

Smart contract developers working on Ethereum mainnet use assembly instead of regular Solidity to save gas thereby reducing transaction fees for users. However using assembly instead of normal Solidity can introduce subtle memory corruption issues and result in unexpected behavior that smart contract auditors and developers should be aware of.

Prerequisite Knowledge

In order to understand this deep dive you'll need to have completed at least Section 1 of Updraft's Assembly & Formal Verification course or already have equivalent knowledge of:

Memory Corruption From External Call

The return value of an external call is stored into memory at the Next Free Memory Address (NFMA), which is retrieved by reading memory at the Free Memory Pointer Address(FMPA - 0x40). If the smart contract developer has not accounted for this, when an external call occurs after an assembly block the return value of the external call can over-write data stored in memory by the preceding assembly block. This can result in insidious vulnerabilities if that data is used by subsequent calculations.

To see exactly how this memory corruption occurs examine this simplified stand-alone Foundry example based upon this real-world finding:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;

import {Test} from "forge-std/Test.sol";

// separate contracts accessed via interfaces
// prevents the optimizer from being "too smart", helping
// to better approximate real-world execution
interface IInfoRetriever {
    function getVal() external returns(uint256);
}

// this contract gets used to return additional
// values required by the primary computation
contract InfoRetriever is IInfoRetriever {
    uint256 returnVal = 3;

    function setVal(uint256 newVal) public {
        returnVal = newVal;
    }

    function getVal() external view returns(uint256) {
       return returnVal;
    }
}

interface IHasher {
    function hashInputs(uint256 a, uint256 b, uint256 additionalInputCount) external returns(bytes32 result);
}

// actual implementation that computes a hash of
// two primary inputs and an optional number of
// secondary inputs
contract HasherImpl is IHasher {

    // used to retrieve optional number of secondary inputs
    IInfoRetriever infoRetriever;

    constructor(IInfoRetriever _infoRetriever) {
        infoRetriever = _infoRetriever;
    }

    function _getSecondaryInputs(uint256 dataPtr, uint256 inputsToFetch) private returns(uint256) {
        for(uint256 i; i<inputsToFetch; i++) {
            // make external call to retrieve the data
            uint256 input = infoRetriever.getVal();

            assembly {
                // for each additional input, store it into memory
                // at next free memory address
                mstore(dataPtr, input)

                // update pointer to subsequent next free memory address
                dataPtr := add(dataPtr, 0x20)
            }
        }

        // returns updated dataPtr
        return dataPtr;
    }

    function hashInputs(uint256 a, uint256 b, uint256 additionalInputCount) external returns(bytes32 result) {
        uint256 dataPtr;

        assembly {
            // read free memory pointer to find next free memory address
            dataPtr := mload(0x40)

            // store `a` in memory at next free memory address
            mstore(dataPtr, a)

            // update pointer to subsequent next free memory address
            dataPtr := add(dataPtr, 0x20)

            // store `b` in memory at next free memory address
            mstore(dataPtr, b)

            // update pointer to subsequent next free memory address
            dataPtr := add(dataPtr, 0x20)
        }

        // get the additional input from the info retriever, passing the updated
        // `dataPtr` so any new elements will be saved in subsequent free memory
        // addresses
        dataPtr = _getSecondaryInputs(dataPtr, additionalInputCount);

        // compute hash of all inputs
        assembly {
            let startPtr := mload(0x40)
            result := keccak256(startPtr, sub(dataPtr, startPtr))
        }
    }
}

// test harness
contract HasherTest is Test {

    IInfoRetriever infoRetriever = new InfoRetriever();
    IHasher hasher = new HasherImpl(infoRetriever);
    uint256 a = 1;
    uint256 b = 2;

    function test_HashWithoutAdditionalInput() external {
        // works great
        bytes32 result = hasher.hashInputs(a, b, 0);

        assertEq(result, keccak256(abi.encode(1, 2)));
    }

    function test_HashWithAdditionalInput() external {
        // fails due to calculating incorrect hash
        bytes32 result = hasher.hashInputs(a, b, 1);

        assertEq(result, keccak256(abi.encode(1, 2, 3)));   
    }
}

This simplified example contains a contract HasherImpl whose purpose is to:

  • receive a set of primary inputs (a, b)

  • optionally retrieve a set of secondary inputs

  • calculate a hash over the entire set of primary and secondary inputs

HasherImpl uses assembly to:

  • correctly calculate Next Free Memory Addresses (NFMA)

  • correctly store both primary and secondary inputs into NFMAs

  • perform a gas-efficient hash over the entire input set

When no secondary inputs exist everything works correctly which can be verified by forge test --match-contract HasherTest --match-test test_HashWithoutAdditionalInput

However when using one or more secondary inputs the relevant test test_HashWithAdditionalInput fails as an incorrect hash is calculated. This occurs for two reasons - let's examine the first cause in detail!

Manual Assembly Analysis

Examining the assembly code superficially everything looks great; each primary and secondary input is saved into the NFMA and dataPtr is always updated to point to the next NFMA such that no variables will over-write each-other.

However when secondary inputs are retrieved a very subtle bug occurs due to the external call(s) to IInfoRetriever::getVal since:

  • the manual assembly never updated the FMPA (0x40) and stored the first input a at the Starting Next Free Memory Address (SNFMA - 0x80)

  • when setting up for the external call, the FMPA will be loaded from memory and a temporary value used in the external call will be stored at the address it points to (0x80)

  • when the external call completes, the FMPA will again be loaded from memory and the return value stored at the address it points to (0x80)

  • this causes memory corruption at 0x80, over-writing the input variable a both before and after the external call

Hence the hash will never execute over the entire input set since at least one of the inputs was over-written, resulting in an incorrect hash being calculated. Let's prove this is exactly what happens using Foundry's debugger to step through every opcode and see the exact moment when memory corruption occurs!

Opcode Execution Trace

To start the debugger execute forge test --match-contract HasherTest --debug test_HashWithAdditionalInput. We are concerned with the execution inside HasherImpl::hashInputs:

  • from the first PUSH1(0x40) after calldata has been loaded onto the stack and JUMPDEST has been called

  • until the memory corruption occurs

The relevant execution starts at PC 0x56 (86) and looks like this:

// push Free Memory Pointer Address (FMPA) onto stack
PUSH1(0x40) [Stack : 0x40, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]    
            [Memory: 0x40 = 0x80                             ]

// duplicate FMPA
DUP1        [Stack : 0x40, 0x40, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80                                   ]

// load Starting Next Free Memory Address (SNFMA) by reading FMPA
MLOAD       [Stack : 0x80, 0x40, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80                                   ]

// duplicate value of `a` input
DUP5        [Stack : 0x01, 0x80, 0x40, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80                                         ]

// duplicate SNFMA
DUP2        [Stack : 0x80, 0x01, 0x80, 0x40, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80                                               ]

// store value of `a` into memory at SNFMA
MSTORE      [Stack : 0x80, 0x40, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80, 0x80 = 0x01                      ]

// push 0x20 onto stack (2nd param of first `add` call)
// this is an offset to calculate Next Free Memory Address (NFMA)
PUSH1(0x20) [Stack : 0x20, 0x80, 0x40, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80, 0x80 = 0x01                            ]

// duplicate SNFMA
DUP2        [Stack : 0x80, 0x20, 0x80, 0x40, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80, 0x80 = 0x01                                  ]

// calculate NFMA by SNFMA + offset (0x80 + 0x20)
ADD         [Stack : 0xa0, 0x80, 0x40, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80, 0x80 = 0x01                            ]

// duplicate value of `b` input
DUP5        [Stack : 0x02, 0xa0, 0x80, 0x40, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80, 0x80 = 0x01                                  ]

// exchange 2nd and 1st elements
SWAP1       [Stack : 0xa0, 0x02, 0x80, 0x40, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80, 0x80 = 0x01                                  ]

// store value of `b` into memory at NFMA
MSTORE      [Stack : 0x80, 0x40, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80, 0x80 = 0x01, 0xa0 = 0x02         ]

// not sure what the purpose of this is?
PUSH1(0x00) [Stack : 0x00, 0x80, 0x40, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80, 0x80 = 0x01, 0xa0 = 0x02               ]

// exchange 3rd and 1st stack elements
SWAP2       [Stack : 0x40, 0x80, 0x00, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80, 0x80 = 0x01, 0xa0 = 0x02               ]

// calculate NFMA by SNFMA + offset (0x80 + 0x40) 
ADD         [Stack : 0xc0, 0x00, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80, 0x80 = 0x01, 0xa0 = 0x02         ]

// `a` and `b` primary inputs have been written to memory
// and the next free memory address 0xc0 has been calculated and
// on the stack. We skip opcodes concerned with calling internal 
// function `getSecondaryInputs` and evaluating `for` loop condition
// until we reach PC 0xc2 (194) which is setting up for the external
// call
DUP2        [Stack : 0xe1cb0e52..00, 0x80, 0xe1cb0e52..00, 0x5616..b72f, 0x00, 0x00, 0x00, 0x01, 0xc0, 0x71, 0xc0, 0x00, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80, 0x80 = 0x01, 0xa0 = 0x02                                                                                                 ]

// before making external call the value of `a` at 0x80 will be 
// over-written - memory corruption first occurs before external call!
MSTORE      [Stack : 0xe1cb0e52..00, 0x5616..b72f, 0x00, 0x00, 0x00, 0x01, 0xc0, 0x71, 0xc0, 0x00, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80,
                     0x80 = 0xe1cb0e52..00, // `a` over-written
                     0xa0 = 0x02]

// we then move forward into `InfoRetriever::getVal` until `RETURN`
// takes us back into `HasherImpl::_getSecondaryInputs` PC 0x0d5 (213),
// where we observe that 0x80 now contains the return value `3` from
// the external call
ISZERO      [Stack : 0x84, 0xe1cb0e52..00, 0x5616..b72f, 0x00, 0x00, 0x00, 0x01, 0xc0, 0x71, 0xc0, 0x00, 0x01, 0x02, 0x01, 0x43, 0xe98b5f2d]
            [Memory: 0x40 = 0x80,
                     0x80 = 0x03, // `a` over-written again
                     0xa0 = 0x02]

// manual assembly to store `3` gets executed at PC 0x10b (267)
// which stores the return value of the external call at 0xc0.
// however the return value `3` was also stored at 0x80 where `a` was 
// previously stored so the correct hash can no longer be computed.
// also note that solidity has updated the FMPA 0x40 to point to
// 0xa0 which is where we are storing the value of variable `b` meaning
// that the second input variable could also be over-written
MSTORE      [Memory: 0x40 = 0xa0, // FMPA updated to where `b` stored!
                     0x80 = 0x03, // `a` remains over-written
                     0xa0 = 0x02, // `b` correct
                     0xc0 = 0x03  // manual assembly stores return val]

The above opcode trace shows:

  • the value of input variable a stored in memory at 0x80 gets over-written twice; once before and once after the external call

  • Solidity has updated the FMPA to point to 0xa0 which is where the manual assembly is storing the value of input variable b - this makes it possible for future operations to also over-write the second input variable

  • it is impossible to now calculate the correct hash as at least one input variable a has been over-written due to memory corruption caused by the manual assembly not accounting for how Solidity uses memory during the external function call

To fix this problem we need to modify the first block in our manual assembly to update the Free Memory Pointer Address (FMPA) to point to the Next Free Memory Address (NFMA) like so:

function hashInputs(uint256 a, uint256 b, uint256 additionalInputCount) external returns(bytes32 result) {
    uint256 dataPtr;

    assembly {
        // read free memory pointer to find next free memory address
        dataPtr := mload(0x40)

        // store `a` in memory at next free memory address
        mstore(dataPtr, a)

        // update pointer to subsequent next free memory address
        dataPtr := add(dataPtr, 0x20)

        // store `b` in memory at next free memory address
        mstore(dataPtr, b)

        // update pointer to subsequent next free memory address
        dataPtr := add(dataPtr, 0x20)

        // @audit this prevents the external call in `_getSecondaryInputs`
        // from over-writing value of `a` stored at 0x80
        //
        // update free memory pointer to point to next free memory address
        mstore(0x40, dataPtr)
    }

Making this change then re-running the Foundry debugger shows at PC 0x110 (272):

  • the return value of the external call is stored in memory at 0xc0 with both primary and secondary input variables having been correctly stored in memory

  • the FMPA 0x40 points to unused 0xe0 such that any new values stored in memory won't over-write the primary and secondary input variables required for the hash calculation

MSTORE      [Memory: 0x40 = 0xe0, // FMPA updated to unused value
                     0x80 = 0x01, // `a` correct
                     0xa0 = 0x02, // `b` correct
                     0xc0 = 0x03] // returned secondary input correct

However this is not enough as another subtle bug lies in wait to corrupt our hash calculation!

Assuming Unchanged Free Memory Pointer Address

Another subtle problem when mixing blocks of manual assembly with normal Solidity code is when the manual assembly code assumes that the Free Memory Pointer Address (FMPA) hasn't changed simply because it hasn't changed it - this is a dangerous assumption because the normal Solidity code in between manual assembly blocks could have updated the FMPA.

In the previous opcode execution trace we observed that:

  • even before making our fix for the identified bug, Solidity updated the FMPA after returning the secondary input value from the external call

  • our fix required a manual update to the FMPA before the external call

Any update to the FMPA is problematic for our code because the manual assembly block which calculates the hash:

  • reads the Next Free Memory Address (NFMA) from the current FMPA

  • assumes that the inputs to be hashed start at that NFMA

// compute hash of all inputs
assembly {
    // @audit assumes FMPA unchanged from first assembly block
    // where primary inputs are stored my first loading FMPA
    let startPtr := mload(0x40)
    result := keccak256(startPtr, sub(dataPtr, startPtr))
}

Hence once the external function which fetches the secondary inputs executes and the FMPA has been updated, the hashing code will never correctly hash the appropriate inputs even if memory corruption didn't occur. We can observe this in the opcode execution traces, firstly for the original version without our first fix starting from PC 0x079 (121):

// exchange 2nd and 1st stack elements
SWAP1       [Stack : 0xa0, 0x40, ......]    
            [Memory: 0x40 = 0xa0,
                     0x80 = 0x03, // value of 'a' corrupted by ext func
                     0xa0 = 0x02,
                     0xc0 = 0x03]

// call keccak256(offset, size)
// this will hash memory 0xa0 and 0xc0
// returning keccak256(abi.encode(2,3))
KECCAK256(0xa0, 0x40)

Secondly the same error occurs in our "fixed" version where the memory corruption issue has been resolved starting from PC 0x7e (126):

// exchange 2nd and 1st stack elements
SWAP1       [Stack : 0xe0, 0x00, ......]    
            [Memory: 0x40 = 0xe0,
                     0x80 = 0x01, // no memory corruption
                     0xa0 = 0x02,
                     0xc0 = 0x03]

// call keccak256(offset, size)
// this will hash empty memory since now FMPA points to unused memory
KECCAK256(0xa0, 0x00)

To resolve the second issue:

  • the first manual assembly block needs to save the starting memory address for where the input variables are stored

  • before the external function call the FMPA needs to be updated to an unused address (our first fix)

  • the final assembly block needs to use the saved starting memory address to calculate the offset and size parameters passed to keccak256

The full fixed source code contains @audit tags to indicate all changes made:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;

import {Test} from "forge-std/Test.sol";

// separate contracts accessed via interfaces
// prevents the optimizer from being "too smart", helping
// to better approximate real-world execution
interface IInfoRetriever {
    function getVal() external returns(uint256);
}

// this contract gets used to return additional
// values required by the primary computation
contract InfoRetriever is IInfoRetriever {
    uint256 returnVal = 3;

    function setVal(uint256 newVal) public {
        returnVal = newVal;
    }

    function getVal() external view returns(uint256) {
       return returnVal;
    }
}

interface IHasher {
    function hashInputs(uint256 a, uint256 b, uint256 additionalInputCount) external returns(bytes32 result);
}

// actual implementation that computes a hash of
// two primary inputs and an optional number of
// secondary inputs
contract HasherImpl is IHasher {

    // used to retrieve optional number of secondary inputs
    IInfoRetriever infoRetriever;

    constructor(IInfoRetriever _infoRetriever) {
        infoRetriever = _infoRetriever;
    }

    function _getSecondaryInputs(uint256 dataPtr, uint256 inputsToFetch) private returns(uint256) {
        for(uint256 i; i<inputsToFetch; i++) {

            // @audit once this external call completes it will:
            // - read the free memory pointer to find next free memory address
            //   which will be 0x80 as we never updated it since we are manually
            //   allocating memory
            // - write the result of the external call to this memory address
            // - but this is where the input variable `a` was saved!
            // - hence `a` gets subtly over-written by the return
            //   value of this external call which results in the hash calculation
            //   becoming corrupted!
            //
            // make external call to retrieve the data
            uint256 input = infoRetriever.getVal();

            assembly {
                // for each additional input, store it into memory
                // at next free memory address
                mstore(dataPtr, input)

                // update pointer to subsequent next free memory address
                dataPtr := add(dataPtr, 0x20)
            }
        }

        // returns updated dataPtr
        return dataPtr;
    }

    function hashInputs(uint256 a, uint256 b, uint256 additionalInputCount) external returns(bytes32 result) {
        uint256 dataPtr;

        // @audit used to hash correct memory addresses at the end
        uint256 startDataPtr;

        assembly {
            // read free memory pointer to find next free memory address
            dataPtr := mload(0x40)

            // @audit save starting address where we save our variables
            // as 0x40 gets overwritten later 
            startDataPtr := dataPtr

            // store `a` in memory at next free memory address
            mstore(dataPtr, a)

            // update pointer to subsequent next free memory address
            dataPtr := add(dataPtr, 0x20)

            // store `b` in memory at next free memory address
            mstore(dataPtr, b)

            // update pointer to subsequent next free memory address
            dataPtr := add(dataPtr, 0x20)

            // @audit this prevents the external call in `_getSecondaryInputs` from
            // over-writing value of `a` stored at 0x80
            //
            // update free memory pointer to point to next free memory address
            mstore(0x40, dataPtr)
        }

        // get the additional input from the info retriever, passing the updated
        // `dataPtr` so any new elements will be saved in subsequent free memory
        // addresses
        dataPtr = _getSecondaryInputs(dataPtr, additionalInputCount);

        // compute hash of all inputs
        assembly {
            // @audit using saved starting memory address
            result := keccak256(startDataPtr, sub(dataPtr, startDataPtr))
        }
    }
}

// test harness
contract HasherTest is Test {

    IInfoRetriever infoRetriever = new InfoRetriever();
    IHasher hasher = new HasherImpl(infoRetriever);
    uint256 a = 1;
    uint256 b = 2;

    function test_HashWithoutAdditionalInput() external {
        // works great
        bytes32 result = hasher.hashInputs(a, b, 0);

        assertEq(result, keccak256(abi.encode(1, 2)));
    }

    function test_HashWithAdditionalInput() external {
        // now also works
        bytes32 result = hasher.hashInputs(a, b, 1);

        assertEq(result, keccak256(abi.encode(1, 2, 3)));

        // also works for more than 1 secondary input
        result = hasher.hashInputs(a, b, 3);

        assertEq(result, keccak256(abi.encode(1, 2, 3, 3, 3)));
    }
}

Verify that both tests pass using forge test --match-contract HasherTest .

Memory Corruption Due To Insufficient Allocation

Another subtle memory corruption issue that can occur is due to insufficient allocation, a subsequent memory variable can become corrupted when the variable before it in memory is updated. This is likely to occur if there is an asymmetry between functions which allocate capacity and functions which write. Consider this real-world insufficient allocation memory corruption from Ethereum Name Service (ENS) smart contract audit which I've simplified into the following stand-alone Foundry test harness:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;

import {Test} from "forge-std/Test.sol";

// finding   : https://solodit.xyz/issues/memory-corruption-in-buffer-fixed-consensys-ens-permanent-registrar-markdown
// code from : https://github.com/ensdomains/buffer/blob/c06d796e2f6086473d229a96c2eb75053a19b8ec/contracts/Buffer.sol
library Buffer {

    struct buffer {
        bytes buf;
        uint capacity;
    }

    function init(buffer memory buf, uint capacity) internal pure returns(buffer memory) {
        if (capacity % 32 != 0) {
            capacity += 32 - (capacity % 32);
        }
        // Allocate space for the buffer data
        buf.capacity = capacity;
        assembly {
            let ptr := mload(0x40)
            mstore(buf, ptr)
            mstore(ptr, 0)

            // @audit does not account for length 0x20 (32),
            // even though the `write` function does
            mstore(0x40, add(ptr, capacity))
            // @audit fix:
            //mstore(0x40, add(32, add(ptr, capacity)))
        }
        return buf;
    }

    function append(buffer memory buf, bytes memory data) internal pure returns (buffer memory) {
        return write(buf, buf.buf.length, bytes32(data), data.length);
    }

    function write(buffer memory buf, uint off, bytes32 data, uint len) private pure returns(buffer memory) {
        if (len + off > buf.capacity) {
            resize(buf, max(buf.capacity, len) * 2);
        }

        uint mask = 256 ** len - 1;
        // Right-align data
        data = data >> (8 * (32 - len));
        assembly {
            // Memory address of the buffer data
            let bufptr := mload(buf)
            // Address = buffer address + sizeof(buffer length) + off + len
            let dest := add(add(bufptr, off), len)
            mstore(dest, or(and(mload(dest), not(mask)), data))
            // Update buffer length if we extended it
            if gt(add(off, len), mload(bufptr)) {
                mstore(bufptr, add(off, len))
            }
        }
        return buf;
    }

    function resize(buffer memory buf, uint capacity) private pure {
        bytes memory oldbuf = buf.buf;
        init(buf, capacity);
        append(buf, oldbuf);
    }

    function max(uint a, uint b) private pure returns(uint) {
        if (a > b) {
            return a;
        }
        return b;
    }

}

// test harness
contract BufferTest is Test {
    using Buffer for Buffer.buffer;

    function test_MemoryCorruption() external {
        Buffer.buffer memory buffer;
        buffer.init(1);

        // `foo` immediately follows buffer.buf in memory
        bytes memory foo = new bytes(0x01);

        // sanity check passes
        assert(1 == foo.length);

        // append "A" 0x41 (65) to buffer. This gets written to
        // the high order byte of `foo.length`!
        buffer.append("A");

        // foo.length == 0x4100000000000000000000000000000000000000000000000000000000000001
        //            == 29400335157912315244266070412362164103369332044010299463143527189509193072641
        // this passes showing the memory corruption
        assertEq(29400335157912315244266070412362164103369332044010299463143527189509193072641, 
                 foo.length);
    }
}

Run the test in Foundry's debugger using forge test --match-contract BufferTest --debug test_MemoryCorruption and let's examine how this memory corruption happens:

// PC 0x9b6 (2486) executing this line in `Buffer::init`:
// mstore(0x40, add(ptr, capacity))
// updates the Free Memory Pointer Address (FMPA) to 0x120
MSTORE(0x40, 0x120)
Memory: [0x40 = 0x120, // FMPA updated
         0x80 = 0x100,
         0xa0 = 0x20,
         0xc0 = 0x60]

// PC 0x697 (1687) executing this line in `BufferTest::test_Mem...`
// bytes memory foo = new bytes(0x01);
// writes `foo.length` to 0x120
MSTORE(0x120, 0x01)
Memory: [0x40 = 0x120,
         0x80 = 0x100,
         0xa0 = 0x20,
         0xc0 = 0x60,
         0x120 = 0x01] // foo.length

// PC 0xbad (2989) executing this line in `Buffer::write`
// mstore(dest, or(and(mload(dest), not(mask)), data))
// over-writes high-order byte of `foo.length`
MSTORE(0x101, 0x41)
Memory: [0x40 = 0x120,
         0x80 = 0x100,
         0xa0 = 0x20,
         0xc0 = 0x60,
         0x120 = 0x4100..01] // foo.length memory corrupted

Applying the suggested fix results in foo.length being written to 0x140 which prevents the memory corruption. Similar findings: [1, 2]

External Call To Non-Existent Contract

When using low-level call, a call to an address without code deployed is always successful. Consider the following code based upon this real-world mainnet finding:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;

import {Test} from "forge-std/Test.sol";

// external smart contract wallet which verifies a wallet's signature
// for a given hash
interface IWallet {
    function isValidSignature(bytes32 _hash, bytes calldata signature) external returns (bool);
}

// separate contracts accessed via interfaces
// prevents the optimizer from being "too smart", helping
// to better approximate real-world execution
interface IWalletVerifier {
    function isValidWalletSignature(bytes32 _hash, address walletAddress, bytes memory signature)
        external view returns(bool);
}

// implements vulnerable function used to make external call
contract WalletVerifier is IWalletVerifier {
    function isValidWalletSignature(bytes32 _hash, address walletAddress, bytes calldata signature)
        external view returns (bool isValid) {

        bytes memory callInput = abi.encodeWithSelector(
            IWallet(walletAddress).isValidSignature.selector,
            _hash,
            signature
        );

        assembly {
            let cdStart := add(callInput, 32)
            let success := staticcall(
                gas(),            // forward all gas
                walletAddress,    // address of Wallet contract
                cdStart,          // pointer to start of input
                mload(callInput), // length of input
                cdStart,          // write output over input
                32                // output size is 32 bytes
            )

            switch success
            case 0 {
                // Revert with `Error("WALLET_ERROR")`
                /* snip */
                revert(0, 100)
            }
            case 1 {
                // Signature is valid if call did not revert and returned true
                isValid := mload(cdStart)
            }
        }
        return isValid;
    }
}

// test harness
contract CallTest is Test {
    IWalletVerifier verifier = new WalletVerifier();

    function test_EOAAddressCallVerifiesInvalidSig() external view {
        bytes32 emptyHash;
        bytes memory emptySignature;

        bool returnVal = verifier.isValidWalletSignature(emptyHash, address(0x1234), emptySignature);

        assert(returnVal);
    }
}

The function WalletVerifier::isValidWalletSignature verifies a signature by calling an external wallet smart contract. But if this function is called passing the address of an Externally Owned Account (EOA - an ordinary user wallet) then it will always return true as:

  • the staticcall to walletAddress will always return 1 for EOA addresses

  • the staticcall specifies the output from the external call should be written over the input memory

  • in this case there is no external output hence the inputs will not be overwritten

  • hence the mload will return valid data which will be converted into true resulting in the return variable isValid being set to true

This would allow an invalid signature to be verified for EOA accounts. To resolve this vulnerability when using low-level calls:

  1. before making the external call, the size of the contract should be checked to ensure it has code:
if iszero(extcodesize(walletAddress)) {
    // Revert with `Error("WALLET_ERROR")`
    revert(0, 100)
}
  1. if the external call was successful, the length of the returned data should be verified to match the expected length:
if iszero(eq(returndatasize(), 32)) {
    // Revert with `Error("WALLET_ERROR")`
    revert(0, 100)
}

Overflow / Underflow During Inline Assembly

Using inline assembly for opcodes such as add and mult does not offer any overflow/underflow protection which normal Solidity provides. Consider the following simplified stand-alone example based on this unique audit contest finding:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;

import {Test} from "forge-std/Test.sol";

// separate contracts accessed via interfaces
// prevents the optimizer from being "too smart", helping
// to better approximate real-world execution
interface IDexPair {
    function getSwapQuote(uint256 amountToSwap) external view returns(uint256);
}

// represents a decentralized exchange pair between
// two assets
contract DexPair is IDexPair {
    // given an amount of input tokens
    // always return +1 output tokens
    function getSwapQuote(uint256 amountToSwap) external view returns(uint256 outputTokens) {
        assembly {
            outputTokens := add(amountToSwap, 1)
        }
    }
}

// test harness
contract OverflowTest is Test {
    IDexPair dexPair = new DexPair();

    function test_SwapQuoteOverflow() external {
        // correct: swapping 1 token returns 2 tokens
        assertEq(2, dexPair.getSwapQuote(1));

        // incorrect: swapping max returns 0 due to overflow
        assertEq(0, dexPair.getSwapQuote(type(uint256).max));
    }
}

In order to safeguard against overflow when using add inline assembly one option is to manually check that the result is not smaller than the input:

function getSwapQuote(uint256 amountToSwap) external view returns(uint256 outputTokens) {
    assembly {
        outputTokens := add(amountToSwap, 1)

        // detect overflow if outputTokens < amountToSwap
        if lt(outputTokens, amountToSwap) { revert(0, 0) }
    }
}

Appropriate overflow/underflow checks also need to be implemented for multiplication and subtraction when using inline assembly.

Uint128 Overflow Evades Detection As In-Line Assembly Uses 256 Bits

Another similar yet more subtle overflow vulnerability can exist when instead of using uint256 the previous example solidity code uses uint128 or smaller - the recommended overflow check mitigation fails to capture this overflow! Consider the following simplified stand-alone example based upon this actual audit finding:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;

import {Test} from "forge-std/Test.sol";

// separate contracts accessed via interfaces
// prevents the optimizer from being "too smart", helping
// to better approximate real-world execution
interface IDexPair {
    function getSwapQuoteUint128(uint128 amountToSwap) external view returns(uint128 outputTokens);
}

// represents a decentralized exchange pair between
// two assets
contract DexPair is IDexPair {
    // given an amount of input tokens always return +1 output tokens
    // capping max input to uint128 and using explicit code to
    // detect overflow 
    function getSwapQuoteUint128(uint128 amountToSwap) external view returns(uint128 outputTokens) {
        assembly {
            outputTokens := add(amountToSwap, 1)

            // detect overflow if outputTokens < amountToSwap
            if lt(outputTokens, amountToSwap) { revert(0, 0) }
        }
    }
}

// test harness
contract OverflowTest is Test {
    IDexPair dexPair = new DexPair();

    function test_SwapQuoteUint128Overflow() external {
        // correct: swapping 1 token returns 2 tokens
        assertEq(2, dexPair.getSwapQuoteUint128(1));

        // incorrect: swapping max uint128 returns 0 due to overflow
        // assembly overflow check fails to capture this!
        assertEq(0, dexPair.getSwapQuoteUint128(type(uint128).max));
    }
}

Even though DexPair::getSwapQuoteUint128 restricts both its input and output parameters to uint128 , the in-line assembly always operates on 256 bit values. Hence:

  • the add opcode always returns a 256 bit value

  • even if type(uint128).max is passed as input for amountToSwap, inside the in-line assembly block the value of outputTokens will be represented in 256 bits and therefore greater than the input amountToSwap

  • therefore the overflow detection using the lt opcode will fail to detect the overflow, but the overflow still occurs after the in-line assembly block causing the function to return an incorrect value

One option when working with numbers smaller than uint256 is to use addmod to prevent the use of the full 256 bits:

function getSwapQuoteUint128(uint128 amountToSwap) external view returns(uint128 outputTokens) {
    assembly {
        // use addmod(a,b,N) with N = type(uint128).max
        // to prevent use of full 256 bits
        outputTokens := addmod(amountToSwap, 1, 340282366920938463463374607431768211455)

        // detect overflow if outputTokens < amountToSwap
        if lt(outputTokens, amountToSwap) { revert(0, 0) }
    }
}

Another option to detect these more subtle overflows is to include a check using normal Solidity after the in-line assembly block:

function getSwapQuoteUint128(uint128 amountToSwap) external view returns(uint128 outputTokens) {
    assembly {
        outputTokens := add(amountToSwap, 1)
    }

    // detect overflow outside in-line assembly to
    // prevent uint128 overflow not being detected due to
    // in-line assembly working with 256 bit values
    require(outputTokens >= amountToSwap, "Overflow detected!");
}

Additional Resources

EthereumSoliditySmart ContractsWeb3