Taiko - t4sk's results

Lines of code

https://github.com/code-423n4/2024-03-taiko/blob/b6885955903c4ec6a0d72ebb79b124c6d0a1002b/packages/protocol/contracts/team/airdrop/ERC20Airdrop2.sol#L88-L94 https://github.com/code-423n4/2024-03-taiko/blob/b6885955903c4ec6a0d72ebb79b124c6d0a1002b/packages/protocol/contracts/team/airdrop/ERC20Airdrop2.sol#L40 https://github.com/code-423n4/2024-03-taiko/blob/b6885955903c4ec6a0d72ebb79b124c6d0a1002b/packages/protocol/contracts/team/airdrop/ERC20Airdrop2.sol#L117-L118

Vulnerability details

Impact

Users cannot claim full amount hence leaving dust locked in the contract.

Proof of Concept

Function withdraw can only be called between the time starting at claimEnd and before claimEnd + witndrawalWindow.

100% of airdrop can be claimed at or after claimEnd + witndrawalWindow.

So to claim 100% of the airdrop, user's transcation must be processed at timestamp equal to claimEnd + witndrawalWindow. Otherwise unclaimable dust remains.

But it's merely impossible for users to submit transaction so that it will be executed at a precise time.

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

import "forge-std/src/Test.sol";
import "forge-std/src/console2.sol";

// forge test --match-path test/poc/air-drop-dust.sol -vvv
contract AirDrop {
    uint64 public claimEnd = uint64(block.timestamp + 2 days);
    uint64 public withdrawalWindow = 8 days;

    mapping(address addr => uint256 amountClaimed) public claimedAmount;
    mapping(address addr => uint256 amountWithdrawn) public withdrawnAmount;

    error WITHDRAWALS_NOT_ONGOING();

    modifier ongoingWithdrawals() {
        if (claimEnd > block.timestamp || claimEnd + withdrawalWindow < block.timestamp) {
            revert WITHDRAWALS_NOT_ONGOING();
        }
        _;
    }

    constructor() {
        claimedAmount[msg.sender] = 1e18;
    }

    function withdraw(address user) external ongoingWithdrawals {
        (, uint256 amount) = getBalance(user);
        withdrawnAmount[user] += amount;
    }

    function getBalance(address user)
        public
        view
        returns (uint256 balance, uint256 withdrawableAmount)
    {
        balance = claimedAmount[user];
        // If balance is 0 then there is no balance and withdrawable amount
        if (balance == 0) return (0, 0);
        // Balance might be positive before end of claiming (claimEnd - if claimed already) but
        // withdrawable is 0.
        if (block.timestamp < claimEnd) return (balance, 0);

        // Hard cap timestamp - so range cannot go over - to get more allocation over time.
        uint256 timeBasedAllowance = balance
            * (min(block.timestamp, claimEnd + withdrawalWindow) - claimEnd) / withdrawalWindow;

        withdrawableAmount = timeBasedAllowance - withdrawnAmount[user];
    }

    function min(uint256 a, uint256 b) internal pure returns (uint256) {
        return a <= b ? a : b;
    }
}

contract AirDropTest is Test {
    AirDrop private air_drop;
    uint256 private deadline;

    function setUp() public {
        air_drop = new AirDrop();
        deadline = uint256(air_drop.claimEnd() + air_drop.withdrawalWindow());
    }

    function test_dust() public {
        vm.warp(deadline - 1);
        air_drop.withdraw(address(this));

        uint256 withdrawn = air_drop.withdrawnAmount(address(this));
        console2.log("withdrawn", withdrawn);
    }

    function test_revert_after_deadline() public {
        vm.warp(deadline + 1);
        vm.expectRevert();
        air_drop.withdraw(address(this));

        // uint256 withdrawn = air_drop.withdrawnAmount(address(this));
        // console2.log("withdrawn", withdrawn);
    }
}

Command

forge test --match-path test/poc/air-drop-dust.sol -vvv

Output - claiming before expiry leaves dust. Claiming after expiry reverts.

[PASS] test_dust() (gas: 42605)
Logs:
  withdrawn 999998553240740740

[PASS] test_revert_after_deadline() (gas: 13269)
Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 670.06µs (241.28µs CPU time)

Tools Used

Foundry

Recommended Mitigation Steps

Extend withdrawable period, for example by 10 days

    modifier ongoingWithdrawals() {
        if (claimEnd > block.timestamp || claimEnd + withdrawalWindow + 10 days < block.timestamp) {
            revert WITHDRAWALS_NOT_ONGOING();
        }
        _;
    }

Assessed type

Timing

Transient storage should target base chain instead of chain id = 1

https://github.com/code-423n4/2024-03-taiko/blob/b6885955903c4ec6a0d72ebb79b124c6d0a1002b/packages/protocol/contracts/common/EssentialContract.sol#L120-L123

https://github.com/code-423n4/2024-03-taiko/blob/b6885955903c4ec6a0d72ebb79b124c6d0a1002b/packages/protocol/contracts/common/EssentialContract.sol#L131-L134

TaikoL1 can be deployed on L2 as a base contract for L3. Hence when the reentrancy lock is loaded using transient storage, it should compare chain id of base chain, instead of chain id = 1.

Same for Bridge.sol

https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/bridge/Bridge.sol#L542

https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/bridge/Bridge.sol#L556

For example

        if (block.chainid == BASE_CHAIN_ID) {
            assembly {
                tstore(_REENTRY_SLOT, _reentry)
            }
        }

This BASE_CHAIN_ID can be initialized as an immutable variable.

Cross chain signature replay attack

https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/team/TimelockTokenPool.sol#L170-L171

Same signature can be used to claim grants on multiple chain, if the contract is deployed on multiple chains. It is recommend to hash chain id and address of the contract.

        bytes32 hash = keccak256(abi.encodePacked("Withdraw unlocked Taiko token to: ", _to, block.chainid, address(this)));

Bridge - Incorrect gap count

https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/bridge/Bridge.sol#L38-L48

https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/bridge/IBridge.sol#L60-L64

Context is packed into 2 slots (when used in storage) so the gap is off by one. Correct code is below

    /// @dev Slots 3 and 4.
    Context private __ctx;

    /// @notice Mapping to store banned addresses.
    /// @dev Slot 5.
    mapping(address addr => bool banned) public addressBanned;

    /// @notice Mapping to store the proof receipt of a message from its hash.
    /// @dev Slot 6.
    mapping(bytes32 msgHash => ProofReceipt receipt) public proofReceipt;

    uint256[44] private __gap;