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

`DeliveryPlace::settleAskTaker` Checks Wrong Address When Verifying Permissions

izuman

Summary

DeliveryPlace::settleAskTaker function checks the wrong address which will cause three things:

  1. Bid Maker offer will be able to settle his own offer

  2. Ask Taker will not be able to settle.

  3. Possible loss of funds.

Vulnerability Details

The specific line of code is here

Easiest way to explain is with an example PoC and please read the description after the PoC to explain the steps:
Paste the following test into PreMarkets.t.sol and run with forge test --mt test_settle_ask_taker_wrong_address_check -vvv

function test_settle_ask_taker_wrong_address_check() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 0.01 * 1e18, 12000, 300, OfferType.Bid, OfferSettleType.Turbo
)
);
vm.startPrank(user1); //added
mockUSDCToken.approve(address(tokenManager), 10000 * 10 ** 18);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 500);
​
address stock1Addr = GenerateAddress.generateStockAddress(1);
vm.stopPrank();
​
vm.prank(user1);
systemConfig.updateMarket("Backpack", address(mockPointToken), 0.01 * 1e18, block.timestamp - 1, 3600);
​
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
uint256 balance0 = mockPointToken.balanceOf(address(user));
deliveryPlace.settleAskTaker(stock1Addr, 500); // this can be less than 500 not more
uint256 balance1 = mockPointToken.balanceOf(address(user));
​
assertEq(balance0, balance1); // The balance of the mockPointToken for user is `
​
console2.log(
"Trade tax is:", tokenManager.userTokenBalanceMap(address(user), address(weth9), TokenBalanceType.TaxIncome)
);
console2.log(
"claimable amount is: ",
tokenManager.userTokenBalanceMap(address(user), address(weth9), TokenBalanceType.MakerRefund)
);
console2.log(
"Remaining Cash: ",
tokenManager.userTokenBalanceMap(address(user), address(weth9), TokenBalanceType.RemainingCash)
);
console2.log(
"Sales Revenue: ",
tokenManager.userTokenBalanceMap(address(user), address(weth9), TokenBalanceType.SalesRevenue)
);
console2.log(
"Point Token: ",
tokenManager.userTokenBalanceMap(address(user), address(weth9), TokenBalanceType.PointToken)
);
console2.log(
"Referral Bonus : ",
tokenManager.userTokenBalanceMap(address(user), address(weth9), TokenBalanceType.ReferralBonus)
);
vm.stopPrank();
}

  1. In the PoC above user calls createOffer and opens a bid offer which is a buy offer for 1000 points and user is a maker

  2. Next, user1 calls createTaker and becomes a taker promising to sell user 500 points.

  3. Next updateMarket is called and the market status moves to settling
    The following part is where the error occurs:

  4. Next user calls settleAskTaker which does not revert, but it should.

  5. user transfers pointTokens to the contract to settle their own bid

If implemented correctly the code process should be:
4. user1 calls the settleAskTaker
5. user1 transfers pointTokens to the contract to settle their createOffer which was a promise to sell 500 points.

Impact

Protocol will not function correctly, orders will not be settled, and will cause a loss of funds for users.
In the above PoC example user will lose their deposit for the initial createOffer and the actual taker user1 will not be able to settle.

Tools Used

Foundry and manual review

Recommendations

Makes the following changes here in the DeliveryPlace::settleAskTaker function.

- if (_msgSender() != offerInfo.authority) {
+ if (_msgSender() != stockInfo.authority) {
​
Updates

Lead Judging Commences

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

Give us feedback!