Tadle

Tadle
DeFiFoundry
27,750 USDC
View results
Submission Details
Severity: low
Invalid

not all collateral is returned to the offer maker on settling

Summary

In case the offer maker provides all Points on settlement, it is expected that all offer collateral should be returned. However, this is not the case.

Vulnerability Details

The function DeliveryPlace.settleAskTaker() is designed to be called during the settling period by the offer maker only.

if (_msgSender() != offerInfo.authority) {
revert Errors.Unauthorized();
}

The function purpose is for the offer maker to provide the Points tokens to the protocol and to get his/her collateral back. However the calculation for the collateral for refund is wrong. It wrongly assumes that the called is not the offer maker, in fact, only the offer maker(offer.authority) can call settleAskTaker(). The _isMaker arg is passed as false(non offer maker)

uint256 collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,// offerType=Ask
offerInfo.collateralRate,// 120%!
stockInfo.amount,// 10e18
false,// @audit _isMaker param: should be true
Math.Rounding.Floor
);

Looking at the logic inside OfferLibraries.getDepositAmount() it can be seen that only amount is returned, ignoring the collateral calculations

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/libraries/OfferLibraries.sol#L41

if (_offerType == OfferType.Ask && !_isMaker) {
return _amount;
}

In case the collateral deposited was > 100% the remaining collateral will not be returned. This is because the code where the collateral is taken into account is never reached

return
Math.mulDiv(
_amount,
_collateralRate,
Constants.COLLATERAL_RATE_DECIMAL_SCALER,
_rounding
);

Impact

In case of collateral >100%, only 100% will be returned to the offer maker. Example: collateral 120%, returned 100%

Tools Used

Manual review

Recommendations

Change the is_Maker arg value in OfferLibraries.getDepositAmount()

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

uint256 collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,// offerType=Ask
offerInfo.collateralRate,// 100%
stockInfo.amount,// 10e18
- false,
+ true,
Math.Rounding.Floor
);
Updates

Lead Judging Commences

0xnevi Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[invalid] finding-PreMarkets-settleAskTaker-isMaker-false

Invalid, when a taker creates a offer type `StockType.Ask` for a `OfferType.Bid` in protected mode via `createTaker()`, when the `_depositTokenWhenCreateTaker` is invoked, it computes the collateral for the taker to deposit with `(_offerType == OfferType.Bid && !_isMaker)`, in which the collateral will compute based on collateral ratio set by original offer maker as seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/libraries/OfferLibraries.sol#L44-L51). When `settleAskTaker` is invoked, the same condition of `(_offerType == OfferType.Bid && !_isMaker)` is used, so the same computation seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/libraries/OfferLibraries.sol#L44-L51) is invoked, which means collateral refund is correct.

Support

FAQs

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