Tadle

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

Incorrect accounting of point tokens for BID Takers

Summary

When a Maker creates an ASK Offer, they are offering to sell their points at a specified price. Conversely, a Taker might place a BID Order to purchase those points. In this scenario, when the Token Generation Event (TGE) occurs, the Maker is responsible for settling the transaction and providing the tokens. Similarly, the Taker must finalize their bid after the Maker has settled. Once the bid is closed, the Taker should be able to withdraw the points they have acquired. However, due to incorrect accounting logic in the current protocol implementation, Takers are unable to withdraw the points as expected.

Vulnerability Details

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L187
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L109

The issue stems from the closeBidTaker() function within the DeliveryPlace contract. Specifically, on line #187, the address of the collateral token is mistakenly used instead of the address of the points token. In this scenario, the correct approach should be to account for the number of points tokens being purchased by the Taker, ensuring accurate tracking.

tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
makerInfo.tokenAddress, // makerInfo.tokenAddress is collateral token, should be point token
pointTokenAmount
);

The following test case can be added to the existing test file:
NOTE: need to add the following line: import {MarketPlaceStatus} from "../src/interfaces/ISystemConfig.sol";

function testCloseBidTaker() public {
vm.startPrank(user);
uint256 initialBalanceUser = mockUSDCToken.balanceOf(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
address offerAddr = GenerateAddress.generateOfferAddress(0);
address stock1Addr = GenerateAddress.generateStockAddress(1);
address offer1Addr = GenerateAddress.generateOfferAddress(1);
vm.stopPrank();
vm.prank(user2);
preMarktes.createTaker(offerAddr, 500);
vm.startPrank(user1);
systemConfig.updateMarket("Backpack", address(mockPointToken), 0.01 * 1e18, block.timestamp - 1, 3600);
systemConfig.updateMarketPlaceStatus("Backpack", MarketPlaceStatus.AskSettling);
vm.stopPrank();
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
deliveryPlace.settleAskMaker(offerAddr, 500);
vm.stopPrank();
vm.startPrank(user2);
deliveryPlace.closeBidTaker(stock1Addr);
// Shows that incorrect amount is accounted for mockUSDCToken, should be 0
console.log(tokenManager.userTokenBalanceMap(user2, address(mockUSDCToken), TokenBalanceType.PointToken));
}

Impact

Consequently, Takers are unable to withdraw their points tokens. More critically, the flaw allows users to withdraw collateral tokens instead of the intended points tokens, posing a severe risk to the protocol. This vulnerability could lead to substantial financial losses and significantly disrupt the marketplace's overall functionality.

Tools Used

Manual review, Foundry.

Recommendations

Instead of using makerInfo.tokenAddress which is the collateral token address, use the address of the points token.

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year 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.

Give us feedback!