timeout.md

Description

The function allowPacket() in FastSwitchBoard checks are reduntary and can be bypassed by executors by executing any packetId without required number of watcher attestations . This bug is estimated to be high severity due to "Unauthorized access" & "Cryptographic Flaws".

Where: https://github.com/SocketDotTech/socket-DL/blob/master/contracts/switchboard/default-switchboards/FastSwitchboard.sol#L95

Where: https://github.com/SocketDotTech/socket-DL/blob/master/contracts/switchboard/default-switchboards/OptimisticSwitchboard.sol#L43

Steps to reproduce

1. Assume packedId is invalid (isPacketValid[packetId_] returns false)
2. Assume block.timestamp - proposeTime > timeOutInSeconds
3. Now pass in the paramters to the function call. IT'll return true for a packetId which is not attested by enough number of watchers.
4. Hence by doing this, the executors can bypass checks and write any packet to any plug (basically even can break our lootbox).

Expected behavior

The code should be:

function allowPacket(
        bytes32,
        bytes32 packetId_,
        uint32 srcChainSlug_,
        uint256 proposeTime_
    ) external view override returns (bool) {
        if (tripGlobalFuse || tripSinglePath[srcChainSlug_]) return false;
        if (isPacketValid[packetId_]) return true;
        if (block.timestamp - proposeTime_ > timeoutInSeconds) return true;
        return false;
    }

Actual behavior

The current allowPacket function returns true for a packetId whose difference between the currentTime and proposeTime is greater than timeout.
This breaks the system since switchboards are expected to allowPackets only if enough number of watchers signs a packet. This makes the entire protocol open to attacks from executors.

Fix Suggestions

Move the timeout checks and packetValid checks into one if statement with AND operator. Also the timeout check is now wrong, it should check the opposite. It checks if the difference in time is greater than the allowed timeout instead of checking if the difference is within the allowed timeout.

function allowPacket(
        bytes32,
        bytes32 packetId_,
        uint32 srcChainSlug_,
        uint256 proposeTime_
    ) external view override returns (bool) {
        if (tripGlobalFuse || tripSinglePath[srcChainSlug_]) return false;
        if (isPacketValid[packetId_] && block.timestamp - proposeTime_ < timeoutInSeconds) return true;
        return false;
    }