First Flight #3: Thunder Loan
First Flight #3
Beginner FriendlyFoundryDeFiOracle
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

setAllowedToken can lose user deposits

0xswahili

Summary

Disallowing an AssetToken contract that has user deposits leads to a permanent loss since the address cannot be re-allowed again.

Vulnerability Details

When disallowing an AssetToken contract address, there's no check whether the contract has user deposits. Consequently, the contract becomes delinked and all user deposits it has are lost forever.
POC. Add this test to test/unit/ThunderLoanTest.t.sol

function testCanLoseDeposits() public setAllowedToken hasDeposits {
thunderLoan.setAllowedToken(tokenA, false);
assertEq(address(thunderLoan.getAssetFromToken(tokenA)), address(0));
}

Then run it as:

forge test --match-test testCanLoseDeposits
Running 1 test for test/unit/ThunderLoanTest.t.sol:ThunderLoanTest
[PASS] testCanLoseDeposits() (gas: 1136927)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 109.17ms

Impact

User deposits into a disallowed AssetToken contract are lost forever.

Tools Used

Manual review

Recommendations

Check that the contract to be delisted does not have any user deposits. Add this code at line Ln 240:

+ uint256 currentBalance = token.balanceOf(address(assetToken));
+ uint256 totalSupply = IERC20(address(assetToken)).totalSupply();
+ if(currentBalance > 0 || totalSupply > 0)
+ revert ThunderLoan__AssetTokenHasBalances();
+ delete s_tokenToAssetToken[token];
​
Updates

Lead Judging Commences

0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Admin Input/call validation
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

centralized owners can brick redemptions by unallowing a token

Support

FAQs

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