Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Whitelist Manipulation in TokenManager

juggernaut63

Summary

The updateTokenWhiteListed function allows the owner to add or remove tokens from the whitelist. Attackers can observe pending transactions to update the whitelist and submit their own transactions with higher gas fees to manipulate the whitelist status before the original transaction is processed.

Vulnerability Details

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L197-L209

1.Initial Setup

  • Involved Tokens: TokenA

  • Initial State: TokenA is not in the whitelist (tokenWhiteListed[TokenA] = false).

2.Authorized User Transactions

  • Authorized User: Alice

  • Goal: Add TokenA to the whitelist.

  • Alice's Transactions:
    updateTokenWhiteListed([TokenA], true);

  • Gas Fee: Alice sends a transaction with the default gas fee.

3.Attacker Observes Transactions

  • Attacker: Bob

  • Goal: Disrupt or manipulate the whitelist.

  • Attacker Steps:

  1. Bob observes Alice's transactions in the mempool.

  2. Bob sends a transaction with a higher gas fee to ensure his transaction is processed before Alice's transaction.

4.Attacker Transaction

  • Bob's transaction:
    updateTokenWhiteListed([TokenA], false);

  • Gas Fee: Bob sends a transaction with a higher gas fee than Alice's transaction.

5.Transaction Processing Order (Continued)

  • Bob's transaction is processed first because the gas fee is higher.
    o tokenWhiteListed[TokenA] remains false or is changed to false if it was previously true.
    o Emit event UpdateTokenWhiteListed(tokenAddress: 0x000000000000000000000000000000000000000000003, isWhiteListed: false).

  • Alice's transaction is processed after Bob's transaction.
    o tokenWhiteListed[TokenA] is changed to true.
    o Emit event UpdateTokenWhiteListed(tokenAddress: 0x000000000000000000000000000000000000003, isWhiteListed: true).

Impact

Exploit Impact
• If Bob Removes TokenA from the Whitelist:
o If TokenA is already in the whitelist, Bob can remove it before Alice's transaction re-adds it.
o This could cause confusion or failure in operations that depend on the whitelist status.
• If Bob Adds TokenA to the Whitelist:
o Bob could add TokenA to the whitelist before Alice's transaction, which Alice may not want.

Potential Downsides
• Failed Transactions: Legitimate users' transactions may not work as expected if the whitelist status changes before they are processed.
• Gas Wastage: Legitimate users may have to resubmit transactions at an additional gas cost if the first transaction fails or does not produce the desired result.

Tools Used

  • Manual review

  • Foundry

Recommendations

  • Add a timelock mechanism to slow down changes to the whitelist, giving users time to react to the changes.

mapping(address => uint256) public whitelistChangeTimelock;
​
function updateTokenWhiteListed(address[] calldata _tokens, bool _isWhiteListed) external onlyOwner {
uint256 _tokensLength = _tokens.length;
for (uint256 i = 0; i < _tokensLength; ) {
require(block.timestamp > whitelistChangeTimelock[_tokens[i]], "Timelock not expired");
whitelistChangeTimelock[_tokens[i]] = block.timestamp + 1 days; // Set timelock duration
_updateTokenWhiteListed(_tokens[i], _isWhiteListed);
unchecked {
++i;
}
}
}

  • Use a multi-signature mechanism to validate changes to the whitelist, requiring multiple signatures from the owner or authorized parties.

mapping(address => bool) public isApprover;
mapping(address => uint256) public approvals;
​
modifier onlyApprover() {
require(isApprover[msg.sender], "Not an approver");
_;
}
​
function updateTokenWhiteListed(address[] calldata _tokens, bool _isWhiteListed) external onlyApprover {
uint256 _tokensLength = _tokens.length;
for (uint256 i = 0; i < _tokensLength; ) {
approvals[_tokens[i]]++;
if (approvals[_tokens[i]] >= requiredApprovals) {
_updateTokenWhiteListed(_tokens[i], _isWhiteListed);
approvals[_tokens[i]] = 0; // Reset approvals after successful update
}
unchecked {
++i;
}
}
}
​
function setApprover(address _approver, bool _status) external onlyOwner {
isApprover[_approver] = _status;
}
​
function setRequiredApprovals(uint256 _requiredApprovals) external onlyOwner {
requiredApprovals = _requiredApprovals;
}

  • Limit the frequency of changes to the whitelist to prevent rapid changes and allow users to react.

mapping(address => uint256) public lastWhitelistUpdate;
​
modifier rateLimited(address _token) {
require(block.timestamp >= lastWhitelistUpdate[_token] + 1 days, "Rate limit exceeded");
_;
}
​
function updateTokenWhiteListed(address[] calldata _tokens, bool _isWhiteListed) external onlyOwner {
uint256 _tokensLength = _tokens.length;
for (uint256 i = 0; i < _tokensLength; ) {
_updateTokenWhiteListed(_tokens[i], _isWhiteListed);
lastWhitelistUpdate[_tokens[i]] = block.timestamp;
unchecked {
++i;
}
}

  • Add a mechanism to wait for event confirmation before allowing further changes.

mapping(address => bool) public pendingWhitelistChanges;
​
event WhitelistChangeRequested(address indexed token, bool isWhiteListed);
​
function requestWhitelistChange(address[] calldata _tokens, bool _isWhiteListed) external onlyOwner {
uint256 _tokensLength = _tokens.length;
for (uint256 i = 0; i < _tokensLength; ) {
pendingWhitelistChanges[_tokens[i]] = true;
emit WhitelistChangeRequested(_tokens[i], _isWhiteListed);
unchecked {
++i;
}
}
}
​
function confirmWhitelistChange(address[] calldata _tokens, bool _isWhiteListed) external onlyOwner {
uint256 _tokensLength = _tokens.length;
for (uint256 i = 0; i < _tokensLength; ) {
require(pendingWhitelistChanges[_tokens[i]], "Whitelist change not requested");
_updateTokenWhiteListed(_tokens[i], _isWhiteListed);
pendingWhitelistChanges[_tokens[i]] = false;
unchecked {
++i;
}
}
}

PoC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "forge-std/Test.sol";
import "../src/core/TokenManager.sol";
​
contract ExploitTest is Test {
TokenManager tokenManager;
address alice = address(0x1);
address bob = address(0x2);
address tokenA = address(0x3);
​
function setUp() public {
tokenManager = new TokenManager();
tokenManager.initialize(address(this)); // Assuming the test contract is the owner
tokenManager.transferOwnership(alice); // Transfer ownership to Alice
}
​
function testExploit() public {
// Initial state: TokenA is not in the whitelist
assertFalse(tokenManager.tokenWhiteListed(tokenA));
​
// Bob observes Alice's transaction in the mempool and sends his own transaction with higher gas fee
vm.startPrank(alice);
address[] memory tokensToRemove = new address[](1);
tokensToRemove[0] = tokenA;
tokenManager.updateTokenWhiteListed(tokensToRemove, false);
vm.stopPrank();
​
// Move to the next block
vm.roll(block.number + 1);
​
// Alice's transaction to add TokenA to the whitelist
vm.startPrank(alice);
address[] memory tokensToAdd = new address[](1);
tokensToAdd[0] = tokenA;
tokenManager.updateTokenWhiteListed(tokensToAdd, true);
vm.stopPrank();
​
// Check the final state
bool isTokenAWhiteListed = tokenManager.tokenWhiteListed(tokenA);
assertTrue(isTokenAWhiteListed, "exploit failed: TokenA should be in the whitelist");
}
}

forge test --match-path test/ExploitTest.t.sol
[⠊] Compiling...
[⠰] Compiling 1 files with Solc 0.8.26
[⠒] Solc 0.8.26 finished in 1.32s
Compiler run successful!

Ran 1 test for test/ExploitTest.t.sol:ExploitTest
[PASS] testExploit() (gas: 48680)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 45.99ms (11.12ms CPU time)

Ran 1 test suite in 82.84ms (45.99ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

[invalid] finding-Admin-Errors-Malicious

The following issues and its duplicates are invalid as admin errors/input validation/malicious intents are1 generally considered invalid based on [codehawks guidelines](https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity#findings-that-may-be-invalid). If they deploy/set inputs of the contracts appropriately, there will be no issue. Additionally admins are trusted as noted in READ.ME they can break certain assumption of the code based on their actions, and

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!