Summary
When a user withdraws from the protocol, the mapping holding the amount of tokens a user can withdraw is not updated. This could lead to a malicious user withdrawing the amount he hold over and over untill the contract holding the amount is drained.
Vulnerability Details
The protocol offers the ability for users to trade assets with big bid-ask spreads. This is achieved by leveraging an order-book like functionality, where users can create bids, asks and market takers execute these offers.
When a user opens a new offer the traded tokens are transfered from the user to the CapitalPool where they are stored. This is done via the `TokenManager::tillIn` function:
function createOffer(CreateOfferParams calldata params) external payable {
...
{
uint256 transferAmount = OfferLibraries.getDepositAmount(
params.offerType,
params.collateralRate,
params.amount,
true,
Math.Rounding.Ceil
);
ITokenManager tokenManager = tadleFactory.getTokenManager();
=> tokenManager.tillIn{value: msg.value}(
_msgSender(),
params.tokenAddress,
transferAmount,
false
);
}
...
}
function tillIn(
address _accountAddress,
address _tokenAddress,
uint256 _amount,
bool _isPointToken
)
external
payable
onlyRelatedContracts(tadleFactory, _msgSender())
onlyInTokenWhiteList(_isPointToken, _tokenAddress)
{
...
if (_tokenAddress == wrappedNativeToken) {
* @dev token is native token
* @notice check msg value
* @dev if msg value is less than _amount, revert
* @dev wrap native token and transfer to capital pool
*/
if (msg.value < _amount) {
revert Errors.NotEnoughMsgValue(msg.value, _amount);
}
IWrappedNativeToken(wrappedNativeToken).deposit{value: _amount}();
=> _safe_transfer(wrappedNativeToken, capitalPoolAddr, _amount);
} else {
=> _transfer(
_tokenAddress,
_accountAddress,
=> capitalPoolAddr,
_amount,
capitalPoolAddr
);
}
emit TillIn(_accountAddress, _tokenAddress, _amount, _isPointToken);
}
When an order is settled or closed (funds refunded), the funds are added to users balance in a mapping of user => token => tokenBalanceType => amount
For example the abortAskOffer function:
function abortAskOffer(address _stock, address _offer) external {
...
ITokenManager tokenManager = tadleFactory.getTokenManager();
=> tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
_msgSender(),
makerInfo.tokenAddress,
makerRefundAmount
);
offerInfo.abortOfferStatus = AbortOfferStatus.Aborted;
offerInfo.offerStatus = OfferStatus.Settled;
emit AbortAskOffer(_offer, _msgSender());
}
function addTokenBalance(
TokenBalanceType _tokenBalanceType,
address _accountAddress,
address _tokenAddress,
uint256 _amount
) external onlyRelatedContracts(tadleFactory, _msgSender()) {
=> userTokenBalanceMap[_accountAddress][_tokenAddress][_tokenBalanceType] += _amount;
...
}
Once a user has some belance he can call TokenManager::withdraw to get his funds.
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
);
}
This is the withdraw function. The problem is that when a user withdraws his claimableAmount is not set to 0 afterwards. This lets a malicious user withdraw the same amount over and over untill there is noe left in the Capital pool contract where funds are stored.
Impact
Loss of funds
Tools Used
Manual review
Recommendations
Set the user's claimable amount to 0 when withdrawn. Make sure to do it before the withdraw, because if the token has callback a re-entrancy can happen and the protocol drained.
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
);
}
