Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

`TokenManager::withdraw` allows user to drain `CapitalPool`, as long as he has least 1 of the corresponding token

radin100

Summary

User is able to drain CapitalPool with TokenManager::withdraw, as long as he has least 1 of the corresponding token recorded in TokenManager::userTokenBalanceMap mapping, due to withdrawer's withdrawable balace doesn't reset

Vulnerability Details

Let's see what's happening in scenario where Bob creates a bid offer, abort his offer and refund all his funds:

  1. Firstly Bob creates offer with PreMarkets::createOffer, where he creates offer and deposits X amount of specified token

    function createOffer(CreateOfferParams calldata params) external payable {\
    ....
    ITokenManager tokenManager = tadleFactory.getTokenManager();
    tokenManager.tillIn{value: msg.value}(
    _msgSender(),
    params.tokenAddress,
    transferAmount,
    false
    );
    }
    ​
    ....
    }

  2. Right after that Bob closes his offer with PreMarkets::closeOffer(while offer.usedPoints are still 0). During the execution TokenManager::addBalance is called and now Bob is able to withdraw back X amount of specified token

    function closeOffer(address \_stock, address \_offer) external {\
    ....
    ITokenManager tokenManager = tadleFactory.getTokenManager();
    tokenManager.addTokenBalance(
    TokenBalanceType.MakerRefund,
    _msgSender(),
    makerInfo.tokenAddress,
    refundAmount
    );
    }
    ​
    ....
    }

  3. After that Bob calls TokenManager::withdraw, to get his funds back.

  4. Now Bob received his X tokens, but there is no code that resets his withdrawable balance, which means that Bob is still able to withdraw X tokens again and again until there is no more _tokenAddress tokens in CapitalPool

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

Loss of funds for users, due to withdraw revert, because of not enough tokens in CapitalPool

Tools Used

Manual review

Recommendations

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
.....
+ userTokenBalanceMap[_msgSender()][
+ _tokenAddress
+ ][_tokenBalanceType] = 0;
​
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}
Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-TokenManager-withdraw-userTokenBalanceMap-not-reset

Valid critical severity finding, the lack of clearance of the `userTokenBalanceMap` mapping allows complete draining of the CapitalPool contract. Note: This would require the approval issues highlighted in other issues to be fixed first (i.e. wrong approval address within `_transfer` and lack of approvals within `_safe_transfer_from` during ERC20 withdrawals)

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.