Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Wrong collateralrate used in listoffer could lead to funds being drained.

anonymousjoe

Summary

In the listOffer() function, the user is being made to pay the preoffer's collateral rate instead of the collateral rate passed as the argument to the function. But the offerInfo contains the collateral rate value passed as parameter, this leads to an inconsistancy which a malicious attacker can take advantage off, since in the closeOffer function, the collateral rate mentioned in the offerInfo is taken into account and paid back to the user.

Vulnerability Details

When a user (who owns a "bid" type stock) tries to list an offer in protected mode, he is made to deposit collateral according to the offerInfo.collateralRate (which is the stockInfo.preOffer's collateralRate).

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

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

OfferInfo storage offerInfo = offerInfoMap[stockInfo.preOffer]; // line 313
​
if (makerInfo.offerSettleType == OfferSettleType.Protected) { // line 346
uint256 transferAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate, // here the preoffer's collateral rate is used to deposit
_amount,
true,
Math.Rounding.Ceil
);

But below in the creation of the OfferInfo object, the collateralRate passed as parameter is used.

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

offerInfoMap[offerAddr] = OfferInfo({
id: stockInfo.id,
authority: _msgSender(),
maker: offerInfo.maker,
offerStatus: OfferStatus.Virgin,
offerType: offerInfo.offerType,
abortOfferStatus: AbortOfferStatus.Initialized,
points: stockInfo.points,
amount: _amount,
collateralRate: _collateralRate, // here the collateralrate passed as parameter is used
usedPoints: 0,
tradeTax: 0,
settledPoints: 0,
settledPointTokenAmount: 0,
settledCollateralAmount: 0
});

This is the inconsistancy in the code that a malicious user can take advantage of in many ways:

One possible way is:

  • Assume an ask offer is created with collateralRate = 10,000 (in protected mode).

  • The attacker takes (createTaker) this offer, and thus obtains a "bid" type of stock.

  • The attacker lists the offer (listOffer) this stock, by passing 100,000 as the collateralRate.

  • The attacker, pays collateral corresponding to collateralRate = 10,000, but the offer is made with collateralRate = 100,000 (inconsistancy in the code).

  • The attacker closes the offer (closeOffer)

  • Here the attacker is refunded according to collateralRate = 100,000. This means the attacker has been refunded more than what he deserves.
    https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L437-L456

if ( // closeOffer() function refunding part
makerInfo.offerSettleType == OfferSettleType.Protected ||
stockInfo.preOffer == address(0x0)
) {
uint256 refundAmount = OfferLibraries.getRefundAmount(
offerInfo.offerType,
offerInfo.amount,
offerInfo.points,
offerInfo.usedPoints,
offerInfo.collateralRate // collateralRate = 100,000
);
​
ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
_msgSender(),
makerInfo.tokenAddress,
refundAmount
);
}

Note, the collateralRate(100,000) used is just an example, since there is no upper limit on the collateralRate, the attacker can use a very high collateralRate to completely drain the funds.

Impact

Since the attacker can set the collateralRate himself, he can possibly drain the complete funds of the account.

Tools Used

Manual Review

Recommendations

Update the collateralRate value in listOffer() function to transfer the correct amount (line 349):

if (makerInfo.offerSettleType == OfferSettleType.Protected) {
uint256 transferAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
_collateralRate, // updated collateralRate (line 349)
_amount,
true,
Math.Rounding.Ceil
);
Updates

Lead Judging Commences

0xnevi Lead Judge 10 months 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

Appeal created

anonymousjoe Submitter
10 months ago
0xnevi Lead Judge
10 months ago
0xnevi Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-PreMarkets-listOffer-collateralRate-manipulate

Valid high severity, because the collateral rate utilized when creating an offer is stale and retrieved from a previously set collateral rate, it allows possible manipilation of refund amounts using an inflated collateral rate to drain funds from the CapitalPool contract

Support

FAQs

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