TSender
Cyfrin
DeFiFoundry
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

`TSender::airdropERC20` function does not check for self in `recipients` argument causing lost of some amount of `totalAmount`

EngineAtWork

Summary

The TSender::airdropERC20 function does not check to confirm if the sender doesn't mistakenly include the TSender contract address in the recipients array, causing an irrecoverable loss of some amount out of the totalAmount

Vulnerability Details

The TSender::airdropERC20 function takes an array of addresses (recipients), which is the list of the addresses to which the user wants to airdrop an ERC20 token. The TSender::airdropERC20 does a good amount of checking to ensure that the user doesn't mistakenly airdrop to an unintended address, but the TSender::airdropERC20 function fails to check if the array of addresses (recipients) the sender passes in contains the address of the TSender contract itself. The absence of this check can cause an irrecoverable loss of some amount out of the total amounts the user intended to airdrop. This might seem unlikely to happen, but in fact, it can happen because you need the address of the TSender contract before you can call it to help with airdropping an ERC20 token and in the process of writing a script to call the TSender contract you might mistakenly include the address of the TSender contract in the array of recipients you want to pass to the TSender:airdropERC20 function. Finally, checking for address(0) speaks volumes about how much we care about the funds of the protocol user, and a user mistakenly passing in the address of the TSender contract would result in the same irreversible loss of funds.

POC

Include the below code in Base_Test.t.sol

code
function test_TSender_can_ERC20Token_to_itself() public virtual {
// Arrange
address sender = makeAddr("sender");
uint256 amount = 5 ether;
address[] memory recipients_list = new address[](5);
uint256[] memory ether_value = new uint256[](5);
for(uint160 i = 0; i < 5; ++i) {
if(i == 4) {
recipients_list[i] = address(tSender);
ether_value[i] = 1 ether;
} else {
uint160 index = i + 1;
recipients_list[i] = address(index);
ether_value[i] = 1 ether;
}
}
vm.startPrank(sender);
mockERC20.mint(uint256(amount));
mockERC20.approve(address(tSender), uint256(amount));
vm.stopPrank();
​
// Act
vm.prank(sender);
uint256 startingGas = gasleft();
tSender.airdropERC20(address(mockERC20), recipients_list, ether_value, amount);
uint256 gasUsed = startingGas - gasleft();
console2.log("Gas used", gasUsed);
uint256 mockERC20_balance_of_tSender_contract = mockERC20.balanceOf(address(tSender));
console2.log("The balance of tSender contract on mockERC20 token is: ", mockERC20_balance_of_tSender_contract);
assertEq(mockERC20_balance_of_tSender_contract, 1 ether);
​
}

Then run the command forge test --mt test_TSender_can_ERC20Token_to_itself -vvvv, You will get an output like below showing that indeed the sender successfully airdropped some of the token the TSender contract address.

test output
Traces:
[203244] TSenderReferenceTest::test_TSender_can_ERC20Token_to_itself()
├─ [0] VM::addr(<pk>) [staticcall]
│ └─ ← [Return] sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681]
├─ [0] VM::label(sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681], "sender")
│ └─ ← [Return]
├─ [0] VM::startPrank(sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681])
│ └─ ← [Return]
├─ [46745] MockERC20::mint(5000000000000000000 [5e18])
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681], value: 5000000000000000000 [5e18])
│ └─ ← [Stop]
├─ [24735] MockERC20::approve(TSenderReference: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 5000000000000000000 [5e18])
│ ├─ emit Approval(owner: sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681], spender: TSenderReference: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], value: 5000000000000000000 [5e18])
│ └─ ← [Return] true
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::prank(sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681])
│ └─ ← [Return]
├─ [156698] TSenderReference::airdropERC20(MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], [0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000002, 0x0000000000000000000000000000000000000003, 0x0000000000000000000000000000000000000004, 0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], [1000000000000000000 [1e18], 1000000000000000000 [1e18], 1000000000000000000 [1e18], 1000000000000000000 [1e18], 1000000000000000000 [1e18]], 5000000000000000000 [5e18])
│ ├─ [26062] MockERC20::transferFrom(sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681], TSenderReference: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 5000000000000000000 [5e18])
│ │ ├─ emit Transfer(from: sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681], to: TSenderReference: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], value: 5000000000000000000 [5e18])
│ │ └─ ← [Return] true
│ ├─ [25095] MockERC20::transfer(0x0000000000000000000000000000000000000001, 1000000000000000000 [1e18])
│ │ ├─ emit Transfer(from: TSenderReference: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], to: 0x0000000000000000000000000000000000000001, value: 1000000000000000000 [1e18])
│ │ └─ ← [Return] true
│ ├─ [25095] MockERC20::transfer(0x0000000000000000000000000000000000000002, 1000000000000000000 [1e18])
│ │ ├─ emit Transfer(from: TSenderReference: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], to: 0x0000000000000000000000000000000000000002, value: 1000000000000000000 [1e18])
│ │ └─ ← [Return] true
│ ├─ [25095] MockERC20::transfer(0x0000000000000000000000000000000000000003, 1000000000000000000 [1e18])
│ │ ├─ emit Transfer(from: TSenderReference: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], to: 0x0000000000000000000000000000000000000003, value: 1000000000000000000 [1e18])
│ │ └─ ← [Return] true
│ ├─ [25095] MockERC20::transfer(0x0000000000000000000000000000000000000004, 1000000000000000000 [1e18])
│ │ ├─ emit Transfer(from: TSenderReference: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], to: 0x0000000000000000000000000000000000000004, value: 1000000000000000000 [1e18])
│ │ └─ ← [Return] true
│ ├─ [23095] MockERC20::transfer(TSenderReference: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 1000000000000000000 [1e18])
│ │ ├─ emit Transfer(from: TSenderReference: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], to: TSenderReference: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], value: 1000000000000000000 [1e18])
│ │ └─ ← [Return] true
│ └─ ← [Stop]
├─ [0] console::log("Gas used", 161114 [1.611e5]) [staticcall]
│ └─ ← [Stop]
├─ [559] MockERC20::balanceOf(TSenderReference: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f]) [staticcall]
│ └─ ← [Return] 1000000000000000000 [1e18]
├─ [0] console::log("The balance of tSender contract on mockERC20 token is: ", 1000000000000000000 [1e18]) [staticcall]
│ └─ ← [Stop]
├─ [0] VM::assertEq(1000000000000000000 [1e18], 1000000000000000000 [1e18]) [staticcall]
│ └─ ← [Return]
└─ ← [Stop]
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 40.50ms (3.55ms CPU time)
​
Ran 1 test for test/unit/noCheck/GasliteTest.t.sol:GasliteTest
[PASS] test_TSender_can_ERC20Token_to_itself() (gas: 194902)
Logs:
Gas used 155953
The balance of tSender contract on mockERC20 token is: 1000000000000000000

Impact

This vulnerability would cause the sender to airdrop some of the total amount to the TSender contract. Knowing well enough that the TSender contract doesn't have any function that interacts directly with ERC20 tokens to transfer tokens or approve tokens, it is a fact that any ERC20 token airdrop to the TSender contract is gone forever.

Tools Used

Manual Review

Recommendations

We know well enough that the protocol is all about being hyper-efficient with gas usage for the airdropping activity, But then funds safety and security come first before gas optimization. So we will advise the protocol to implement a check to ensure that the address of the TSender contract is not included in the recipients array and if included the TSender contract should revert.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
kiteweb3 Submitter
over 1 year ago
inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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