Skip to content

Latest commit

 

History

History
162 lines (117 loc) · 4.99 KB

033.md

File metadata and controls

162 lines (117 loc) · 4.99 KB

Magic Laurel Gorilla

Medium

Race Condition in Blacklist Implementation Allows Unrestricted Token Transfers During Blacklist Operation

Summary

The interaction between _onceBlacklisted and _update creates a race condition where a user can perform token transfers during the blacklisting operation due to state checks happening before state updates.

Previous audit found: "Blacklisted accounts can still transact"

They fixed by adding _update checks But they didn't consider the ORDER of operations making their fix incomplete!

https://github.com/sherlock-audit/2024-11-telcoin/blob/main/telcoin-audit/contracts/util/abstract/Blacklist.sol#L77

Root Cause

In Blacklist.sol and Stablecoin.sol, the blacklist state update happens AFTER _onceBlacklistedis called, but _update checks blacklist status. This creates a window where transfers are still possible.

the current implementation:

// In Blacklist.sol
function addBlackList(address user) public virtual onlyRole(BLACKLISTER_ROLE) {
    if (blacklisted(user)) revert AlreadyBlacklisted(user);
    _onceBlacklisted(user);     // Step 1: Seize funds
    _setBlacklist(user, true);  // Step 2: ONLY NOW user is blacklisted
    emit AddedBlacklist(user);
}

// In Stablecoin.sol
function _onceBlacklisted(address user) internal override {
    _transfer(user, _msgSender(), balanceOf(user));  // During this transfer, user is NOT YET blacklisted!
}

function _update(address from, address to, uint256 value) internal override {
    if (blacklisted(from)) revert Blacklisted(from);  // These checks will PASS during _onceBlacklisted
    if (blacklisted(to)) revert Blacklisted(to);      // because user isn't blacklisted yet
    super._update(from, to, value);
}

this is the current flow: Screenshot 2024-11-08 at 3 00 39 PM

THE ISSUE:

Screenshot 2024-11-08 at 3 04 52 PM

REAL WORLD SCENARIO: Alice has 2000 USDC in tokens:

Blacklister sees suspicious activity, calls addBlackList(alice):

  • Contract tries to seize Alice's 2000 USDC
  • But Alice can still transfer because she's not blacklisted yet
  • Alice quickly transfers 1000 USDC to another address

Result:

  1. Blacklister only gets 1000 USDC
  2. Alice saved 1000 USDC from seizure
  3. Alice is now blacklisted but already moved half her funds!

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

  1. Users can still execute transfers during the blacklisting process
  2. Part of the funds can escape seizure
  3. Breaks the atomic nature of blacklist operation
  4. Previous frontrunning mitigation is incomplete

PoC

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.24;

import "forge-std/Test.sol";
import "../src/stablecoin/Stablecoin.sol";

contract BlacklistRaceTest is Test {
    Stablecoin public token;
    address admin = address(0x1);
    address blacklister = address(0x2);
    address user = address(0x3);
    address alice = address(0x4);
    
    function setUp() public {
        token = new Stablecoin();
        token.initialize("Test", "TST", 18);
        
        vm.startPrank(admin);
        token.grantRole(token.BLACKLISTER_ROLE(), blacklister);
        token.grantRole(token.MINTER_ROLE(), admin);
        token.mintTo(user, 2000e18);
        vm.stopPrank();
    }

    function test_raceCondition() public {
        // Setup multicall data for user
        address[] memory targets = new address[](2);
        bytes[] memory data = new bytes[](2);
        
        // First call: Transfer to alice
        targets[0] = address(token);
        data[0] = abi.encodeWithSelector(
            token.transfer.selector,
            alice,
            1000e18
        );
        
        // Second call: Self-transfer remaining balance
        targets[1] = address(token);
        data[1] = abi.encodeWithSelector(
            token.transfer.selector,
            user,
            1000e18
        );
        
        // Blacklister initiates blacklist
        vm.prank(blacklister);
        token.addBlackList(user);
        
        // During _onceBlacklisted execution, user can still transfer!
        // This is because blacklist state isn't set yet
        vm.prank(user);
        token.transfer(alice, 1000e18); // This works!
        
        // Verify final state
        assertEq(token.balanceOf(alice), 1000e18);
        assertEq(token.balanceOf(blacklister), 1000e18);
        assertEq(token.balanceOf(user), 0);
    }
}

Mitigation

  • Move the blacklist state update BEFORE _onceBlacklisted:
function addBlackList(address user) public virtual onlyRole(BLACKLISTER_ROLE) {
    if (blacklisted(user)) revert AlreadyBlacklisted(user);
    _setBlacklist(user, true);  // Set state first
    _onceBlacklisted(user);     // Then seize funds
    emit AddedBlacklist(user);
}