Tadle

Tadle
DeFi
30,000 USDC
View results
Submission Details
Severity: high
Valid

Issue in DeliveryPlace::closeBidTaker Function Leading to Incorrect Token Balance Update

Summary

The closeBidTaker function is called by the Taker after the admin has updated the Token Generation Event (TGE) parameters and the Maker has executed the settleAskMaker function. The purpose of this function is to finalize the bid and ensure that the Taker receives the expected points tokens in their balance. However, an issue has been identified where the addTokenBalance mapping is updated with the wrong token address. Specifically, instead of updating the mapping with the points token address, it is mistakenly updated with the collateral token address. This results in the Taker not receiving the expected points tokens in their balance.

Vulnerability Details

The addTokenBalance mapping is supposed to record the correct amount of points tokens to the Taker’s balance. However, the mapping is mistakenly updated with the collateral token address rather than the points token address. This leads to the points tokens being incorrectly allocated, or not allocated at all, to the Taker’s balance.

function closeBidTaker(address _stock) external {
IPerMarkets perMarkets = tadleFactory.getPerMarkets();
ITokenManager tokenManager = tadleFactory.getTokenManager();
StockInfo memory stockInfo = perMarkets.getStockInfo(_stock);
if (stockInfo.preOffer == address(0x0)) {
revert InvalidStock();
}
if (stockInfo.stockType == StockType.Ask) {
revert InvalidStockType();
}
if (_msgSender() != stockInfo.authority) {
revert Errors.Unauthorized();
}
(OfferInfo memory preOfferInfo, MakerInfo memory makerInfo,,) = getOfferInfo(stockInfo.preOffer);
OfferInfo memory offerInfo;
uint256 userRemainingPoints;
if (makerInfo.offerSettleType == OfferSettleType.Protected) {
offerInfo = preOfferInfo;
userRemainingPoints = stockInfo.points;
} else {
offerInfo = perMarkets.getOfferInfo(makerInfo.originOffer);
if (stockInfo.offer == address(0x0)) {
userRemainingPoints = stockInfo.points;
} else {
OfferInfo memory listOfferInfo = perMarkets.getOfferInfo(stockInfo.offer);
userRemainingPoints = listOfferInfo.points - listOfferInfo.usedPoints;
}
}
if (userRemainingPoints == 0) {
revert InsufficientRemainingPoints();
}
if (offerInfo.offerStatus != OfferStatus.Settled) {
revert InvalidOfferStatus();
}
if (stockInfo.stockStatus != StockStatus.Initialized) {
revert InvalidStockStatus();
}
uint256 collateralFee;
if (offerInfo.usedPoints > offerInfo.settledPoints) {
if (offerInfo.offerStatus == OfferStatus.Virgin) {
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType, offerInfo.collateralRate, offerInfo.amount, true, Math.Rounding.Floor
);
} else {
uint256 usedAmount =
offerInfo.amount.mulDiv(offerInfo.usedPoints, offerInfo.points, Math.Rounding.Floor);
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType, offerInfo.collateralRate, usedAmount, true, Math.Rounding.Floor
);
}
}
uint256 userCollateralFee = collateralFee.mulDiv(userRemainingPoints, offerInfo.usedPoints, Math.Rounding.Floor);
tokenManager.addTokenBalance(
TokenBalanceType.RemainingCash, _msgSender(), makerInfo.tokenAddress, userCollateralFee
);
uint256 pointTokenAmount =
offerInfo.settledPointTokenAmount.mulDiv(userRemainingPoints, offerInfo.usedPoints, Math.Rounding.Floor);
//@audit wrong address
// makerInfo.tokenAddress is the collateral address
// correct address is marketPlaceInfo.tokenAddress
tokenManager.addTokenBalance(
@> TokenBalanceType.PointToken, _msgSender(), makerInfo.tokenAddress, pointTokenAmount
);
perMarkets.updateStockStatus(_stock, StockStatus.Finished);
emit CloseBidTaker(
makerInfo.marketPlace, offerInfo.maker, _stock, _msgSender(), userCollateralFee, pointTokenAmount
);
}

POC

// create a user4

// Run test on test/PreMarkets.t.sol

function test_closeBid() public {
// create offer
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 2000 * 1e18, 12000, 300, OfferType.Ask, OfferSettleType.Turbo
)
);
vm.stopPrank();
// create taker and list offer
vm.startPrank(user3);
address offerAddr = GenerateAddress.generateOfferAddress(0);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr, 1000);
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.listOffer(stock1Addr, 4000 * 1e18, 12_000);
vm.stopPrank();
// create taker
vm.startPrank(user4);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
address offerAddr1 = GenerateAddress.generateOfferAddress(1);
preMarktes.createTaker(offerAddr1, 1000);
vm.stopPrank();
// updated market with Point Token Address, the tokenPerPoint and other variables
vm.prank(user1);
systemConfig.updateMarket("Backpack", address(mockPointToken), 2 * 1e18, block.timestamp - 1, 3600);
// settlement
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 2000 * 10 ** 18);
deliveryPlace.settleAskMaker(offerAddr, 1000);
vm.stopPrank();
// close bid taker
vm.startPrank(user4);
address stock2Addr = GenerateAddress.generateStockAddress(2);
deliveryPlace.closeBidTaker(stock2Addr);
vm.stopPrank();
uint256 user4Balance =
tokenManager.userTokenBalanceMap(user4, address(mockPointToken), TokenBalanceType.PointToken);
// user4 has 0 tokens, test fails
assert(user4Balance > 0);
// test fails
assert(user4Balance == 1000e18);
}

Impact

  1. when the Taker checks their balance after calling closeBidTaker, they do not see the expected points tokens. This discrepancy could cause confusion and financial loss.

  2. The incorrect token allocation could prevent the Taker from fulfilling other contract obligations or participating in subsequent actions that depend on the correct points token balance. This could lead to a cascading failure in the contract’s operation, affecting multiple users and transactions.

Tools Used

Manual Review

Recommendations

Change the wrong token address to the correct one.

Updates

Lead Judging Commences

0xnevi Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-DeliveryPlace-settleAskTaker-closeBidTaker-wrong-makerinfo-token-address-addToken-balance

Valid high severity, In `settleAskTaker/closeBidTaker`, by assigning collateral token to user balance instead of point token, if collateral token is worth more than point, this can cause stealing of other users collateral tokens within the CapitalPool contract, If the opposite occurs, user loses funds based on the points they are supposed to receive

Support

FAQs

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