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

Bid offer points are taken from the maker instead of the taker

dadekuma

Summary

BID offers buy their points instead of receiving them, resulting in a loss of user funds (they pay to get nothing in return).

Vulnerability Details

  1. Alice creates a BID offer for 1000 points and 2000 collateral

  2. Bob creates an ASK order to sell 400 points, he sends 800 collateral

  3. Alice settles 400 points on Bob's order. The problem is, that she must send her own 400 points to close the order

  4. Alice closes her BID offer

  5. Alice paid collateral to receive back her own points. Bob never transferred any points.

Poc, run forge test --via-ir --match-test test_h4_offer_bid_buy_their_own_points -vv

function test_h4_offer_bid_buy_their_own_points() public {
​
address alice = makeAddr("alice");
address bob = makeAddr("bob");
​
// give funds to users
uint256 aliceInitialBalance = 100_000 * 1e18;
deal(address(mockUSDCToken), alice, aliceInitialBalance);
vm.prank(alice);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
​
uint256 bobInitialBalance = 100_000 * 1e18;
deal(address(mockUSDCToken), bob, bobInitialBalance);
​
deal(address(mockUSDCToken), address(capitalPool), bobInitialBalance);
​
vm.prank(bob);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
​
// alice creates offer
vm.startPrank(alice);
​
uint256 collateralAmount = 2000 * 1e18;
​
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000, //points
collateralAmount, //amount
10_000, //collateralRate
0, //eachTradeTax
OfferType.Bid,
OfferSettleType.Protected
)
);
​
address offer0Addr = GenerateAddress.generateOfferAddress(0);
​
// bob is taker
vm.startPrank(bob);
uint256 bobPoints = 400;
preMarktes.createTaker(offer0Addr, bobPoints);
address stock1Addr = GenerateAddress.generateStockAddress(1);
vm.stopPrank();
​
// move market status
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
1e18,
block.timestamp - 1,
3600
);
​
vm.startPrank(alice);
uint256 alicePoints = 10_000 * 1e18;
deal(address(mockPointToken), alice, alicePoints);
mockPointToken.approve(address(tokenManager), type(uint256).max);
​
// alice token balance before is 10_000 and credit is 0:
uint256 tokenAmount;
assertEq(mockPointToken.balanceOf(alice), alicePoints, "alice point token balance");
tokenAmount = tokenManager.userTokenBalanceMap(alice, address(mockUSDCToken), TokenBalanceType.PointToken);
assertEq(tokenAmount, 0, "token alice");
​
// alice has to settle with her own points? but she's buying them?
deliveryPlace.settleAskTaker(stock1Addr, 400);
deliveryPlace.closeBidOffer(offer0Addr);
​
// alice bought her own points, as her point balance is reduced 👍
assertEq(mockPointToken.balanceOf(alice), alicePoints - 400 * 1e18, "alice point token balance");
tokenAmount = tokenManager.userTokenBalanceMap(alice, address(mockUSDCToken), TokenBalanceType.PointToken);
assertEq(tokenAmount, 400 * 1e18, "token alice");
vm.stopPrank();
} // forge test --via-ir --match-test test_h4_offer_bid_buy_their_own_points -vv

Impact

Impact: High (High loss of user funds)
Likelihood: High (It will happen without pre-conditions)

Risk: Critical

Tools Used

Manual Review

Recommendations

In DeliveryPlace, the BID offer maker shouldn't send any tokens, as they are buying them:

if (settledPointTokenAmount > 0) {
- tokenManager.tillIn(
- _msgSender(),
- marketPlaceInfo.tokenAddress,
- settledPointTokenAmount,
- true //isPointToken
- );
​
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
offerInfo.authority,
makerInfo.tokenAddress,
settledPointTokenAmount
);
}

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/DeliveryPlace.sol#L377-L382

Updates

Lead Judging Commences

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

finding-PreMarkets-settleAskTaker-wrong-stock-authority

Valid high severity, when taker offers are created pointing to a `offer`, the relevant `stockInfoMap` offers are created with the owner of the offer aka `authority`, set as the creater of the offer, as seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L245). Because of the wrong check within settleAskTaker, it will permanently DoS the final settlement functionality for taker offers for the maker that listed the original offer, essentially bricking the whole functionality of the market i.e. maker will always get refunded the original collateral, and takers will never be able to transact the original points put up by the maker. This occurs regardless of market mode.

Support

FAQs

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