Tadle

Summary

An attacker can create an offer and once someone takes (createTaker) the offer, the attacker can abort the offer, thus getting all his collateral back and the tradetax as profit.

Vulnerability Details

Assume the following scenario (Bob is the attacker)

• Bob creates an ask offer (createOffer) with the following parameters:

  • points = 100

  • amount = 100USDC

  • collateralRate = 20,000

  • eachTradeTax = 10,000

• This would mean Bob deposits, 200USDC as collateral during the creation of the offer.

• Alice takes the offer (createTaker) and thus deposits the following:

  • depositAmount = 100USDC

  • platformFee = 0.05 USDC (assuming 0.05% platformfee)

  • tradeTax = 100USDC

    https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L212-L234

    uint256 depositAmount = _points.mulDiv(

    offerInfo.amount,

    offerInfo.points,

    Math.Rounding.Ceil

    );

    uint256 platformFee = depositAmount.mulDiv(

    platformFeeRate,

    Constants.PLATFORM_FEE_DECIMAL_SCALER

    );

    uint256 tradeTax = depositAmount.mulDiv(

    makerInfo.eachTradeTax,

    Constants.EACH_TRADE_TAX_DECIMAL_SCALER

    );

    ​

    ITokenManager tokenManager = tadleFactory.getTokenManager();

    _depositTokenWhenCreateTaker(

    platformFee,

    depositAmount,

    tradeTax, // updating Bobs'balance by depositAmount + tradeTax

    makerInfo,

    offerInfo,

    tokenManager

    );

• Thus Bobs balance increases by depositAmount + tradeTax = 200USDC

• Bob proceeds to abort his has offer (abortAskOffer)

  https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L584-L629

• Here transferAmount is calculated as 200USDC (steps mentioned in the code below)

  uint256 remainingAmount;

  if (offerInfo.offerStatus == OfferStatus.Virgin) {

  remainingAmount = offerInfo.amount; // amount = 100USDC

  } else {

  remainingAmount = offerInfo.amount.mulDiv(

  offerInfo.usedPoints,

  offerInfo.points,

  Math.Rounding.Floor

  );

  }

  ​

  uint256 transferAmount = OfferLibraries.getDepositAmount( // 200 USDC (since collateral = 20,000)

  offerInfo.offerType,

  offerInfo.collateralRate,

  remainingAmount,

  true,

  Math.Rounding.Floor

  );

• And the totalDepositAmount = 100USDC

  uint256 totalUsedAmount = offerInfo.amount.mulDiv( // 100USDC (used by Alice)

  offerInfo.usedPoints,

  offerInfo.points,

  Math.Rounding.Ceil

  );

  uint256 totalDepositAmount = OfferLibraries.getDepositAmount( // 100 USDC

  offerInfo.offerType,

  offerInfo.collateralRate,

  totalUsedAmount,

  false,

  Math.Rounding.Ceil

  );

• And the makerRefundAmount = 100USDC which is added to BOB's balance. Thus Bob's current balance becomes 200USDC + 100USDC = 300USDC. (200 USDC was when Alice called the createTakerfunction)

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L616-L629https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L616-L62

Thus, eventhough Bob had deposited only 200USDC (as collateral) he has got 300USDC. This increase is from the tax that Alice paid(tax = 100USDC)

Note: Here an exaggerated amount for tax is used to show the effects of the issue. In reality Bob can make many such ask offers and abort them to have a profit of the tax amount from each of those trades.

Impact

Since malicious users like Bob can use this method to risk nothing and gain a profit of the tax they can create multiple offers and deploy this strategy to gain profits. Thus, users wont be able to trust the listed offers, and will therefore stop buying from them in fear of losing money.

Tools Used

Manual Review

Recommendations

When aborting the offer, subtract the tax from the refund amount. (line 613 abortAskOffer)