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

`DeliveryPlace::closeBidTaker` Adds Wrong Token Balance to Taker Preventing Withdrawal of Point Tokens

schereo

Summary

After settlement, the taker of an ask offer should receive the point tokens they bought. This is done through a token balance which
is added to the taker's account in the TokenManager::addTokenBalance function.
The system incorrectly adds a balance of the collateral token (USDC in the following PoC) to the taker instead of point tokens which the actually bought.

Vulnerability Details

When a taker (buyer) purchases point tokens from a maker (seller), the following process occurs:

  1. The maker creates an offer to sell point tokens (PreMarkets::createOffer)

  2. The taker fills the offer (PreMarkets::createTaker)

  3. The owner updates the market to after the TGE (SystemConfig::updateMarket)

  4. The maker settles the offer (DeliveryPlace::settleAskMaker)

  5. The taker closes the bid taker (DeliveryPlace::closeBidTaker)

In the last step a token balance is added to the taker's account which allows them to withdraw the point tokens they bought later on (DeliveryPlace line 195-200):

// (other existing code)
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
makerInfo.tokenAddress,
pointTokenAmount
);

However, makerInfo.tokenAddress is not the address of the point token but the collateral token. Therefore the TokenManager::addTokenBalance function give the taker the wrong token balance to withdraw (TokenManager line 113-129):

function addTokenBalance(
TokenBalanceType _tokenBalanceType,
address _accountAddress,
address _tokenAddress, // This should be the point token address but is the collateral token address
uint256 _amount
) external onlyRelatedContracts(tadleFactory, _msgSender()) {
userTokenBalanceMap[_accountAddress][_tokenAddress][
_tokenBalanceType
] += _amount;
​
emit AddTokenBalance(
_accountAddress,
_tokenAddress,
_tokenBalanceType,
_amount
);
}

Overview

The following PoC demonstrates the issue. The test will revert because the expected event (allow taker to withdraw 1000 point tokens) does not match the emitted event (allow taker to withdraw 1000 collateral tokens (USDC)).

Actors

  • testSeller: The maker who wants to sell point token

  • testBuyer: The taker who wants to buy point token

  • user1: The owner of the market

Please copy the test into PreMarkets.t.sol:

event AddTokenBalance(
address indexed accountAddress,
address indexed tokenAddress,
TokenBalanceType indexed tokenBalanceType,
uint256 amount
);
​
function testWrongAddTokenBalance() public {
address testSeller = makeAddr("testSeller");
address testBuyer = makeAddr("testBuyer");
// Give maker (seller) the point tokens they want to sell and the collateral
deal(address(mockPointToken), testSeller, 1e19);
deal(address(mockUSDCToken), testSeller, 12e15);
// Give taker (buyer) the tokens needed to buy the points
deal(address(mockUSDCToken), testBuyer, 2e18);
​
// Maker creates offer for 1000 points
vm.startPrank(testSeller);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
mockPointToken.approve(address(tokenManager), type(uint256).max);
​
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask, // Sell points
OfferSettleType.Turbo
)
);
vm.stopPrank();
​
// Taker fulfills the offer
vm.startPrank(testBuyer);
// Approve tokenManager to take the purchase price
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 1000);
vm.stopPrank();
​
// Owner updates the market to after TGE
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
// Maker settles offer
vm.prank(testSeller);
deliveryPlace.settleAskMaker(offerAddr, 1000);
​
// We are expecting an added token balance of 1000 points token to the taker which they just bought
vm.expectEmit(true, true, true, false);
emit AddTokenBalance(testBuyer, address(mockPointToken), TokenBalanceType.PointToken, 1000);
​
// Taker closes bid
vm.startPrank(testBuyer);
address stockAddress = GenerateAddress.generateStockAddress(1);
deliveryPlace.closeBidTaker(stockAddress);
vm.stopPrank();
​
// Maker withdraws their revenue from selling 1000 points
vm.prank(testSeller);
tokenManager.withdraw(
address(mockUSDCToken),
TokenBalanceType.SalesRevenue
);
​
// Taker withdraws the points they bought
vm.prank(testBuyer);
tokenManager.withdraw(
address(mockPointToken),
TokenBalanceType.PointToken
);
}

Impact

  • The taker is unable to withdraw the point tokens they bought, effectively locking their funds in the contract.

  • Probably even more serious: The taker gets a wrong token balance for the collateral token which they can withdraw

    • This disrupts the collateral balance of the system and may prevent others to withdraw their collateral

Impact: High
Likelihood: High

-> Severity: High

Tools Used

  • Manual code review

  • Forge unit tests

Recommendations

The MarketPlaceInfo.tokenAddress contains the address of the point token. Use it instead of the makerInfo.tokenAddress in the DeliveryPlace::closeBidTaker function:

function closeBidTaker() {
// ... (existing code)
​
(
OfferInfo memory preOfferInfo,
MakerInfo memory makerInfo,
+ MarketPlaceInfo memory marketPlaceInfo,
​
) = getOfferInfo(stockInfo.preOffer);
// ... (other existing code)
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
+ marketPlaceInfo.tokenAddress,
- makerInfo.tokenAddress,
pointTokenAmount
);
// ... (remaining code)
}
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!