Tadle

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

`settleAskMaker` stricts check the current `_settledPoints` sent to total used points instead of the current requirement as point token is accepted in multiple passes

Summary

  • The settleAskMaker allows the offer authority to settle points and allows the authority to deposit the points in multiple passes, so they can deposit the whole amount at once or in multiple turns.

  • When full settle amount is sent, it updates the offer status to settled, but if it is settled in multiple passes and even the point tokens amount reaches to the target usedPoints, then also it doesn't give back the collateral to the offer authority due to incorrect check implemented, which only allows collateral refund only if amount is transferred in one go, but as it accepts amount to be transferred in multiple passes then it should refund the collateral token at a pass at which full point token is distributed.

Vulnerability Details

The vulnerability is present in the settleAskMaker function where it doesn't refund the collateral to the offer's authority when point token distributed in multiple passes and only allows collateral refund only if whole amount is transferred in one go when following condition is satisfied.

(_settledPoints == offerInfo.usedPoints)
  • As a result of which the offer's authority will never be able to claim a refund of collateral if they try to send it in multiple passes.
    Here, the offerInfo.usedPoints should be compared with the previous deposit points set via the offerInfo.settledPoints.

  • Also, another vulnerability exist when updating the settledPoints and other related parameters via settledAskOffer, as it doesn't increment the parameters but just updates them leading to incorrect accounting. The parameters will reflect incorrect value and lead to disturbed accounting due to just setting them to value sent instead incrementing them.

  • Also, in settledAskOffer, there is no updation related to settledCollateralAmount which should be updated when collateral is sent.

function settledAskOffer(
address _offer,
uint256 _settledPoints,
uint256 _settledPointTokenAmount
) external onlyDeliveryPlace(tadleFactory, _msgSender()) {
OfferInfo storage offerInfo = offerInfoMap[_offer];
@> offerInfo.settledPoints = _settledPoints;
@> offerInfo.settledPointTokenAmount = _settledPointTokenAmount;
@> offerInfo.offerStatus = OfferStatus.Settled;
emit SettledAskOffer(_offer, _settledPoints, _settledPointTokenAmount);
}

Impact

  • Incorrect parameter updation instead of incrementing in settledAskOffer leads to incorrect accounting.

  • Incorrect check in settleAskMaker to only allow collateral refund only if the whole points are distributed by offer's authority, which leads to not getting the collateral refund when they try to send the point token in multiple passes.

Tools Used

Manual Review

Recommendations

Step 1: Update the parameters in settledAskOffer to be incremented by the values, as below
function settledAskOffer(
address _offer,
uint256 _settledPoints,
uint256 _settledPointTokenAmount
) external onlyDeliveryPlace(tadleFactory, _msgSender()) {
OfferInfo storage offerInfo = offerInfoMap[_offer];
offerInfo.settledPoints += _settledPoints;
offerInfo.settledPointTokenAmount += _settledPointTokenAmount;

offerInfo.offerStatus = OfferStatus.Settled;
emit SettledAskOffer(_offer, _settledPoints, _settledPointTokenAmount);
}

Step 2: In settleAskMaker update the below check as:

- if (_settledPoints == offerInfo.usedPoints) {
+ if (_settledPoints + offerInfo.settledPoints == offerInfo.usedPoints) {
Updates

Lead Judging Commences

0xnevi Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-DeliveryPlace-settleAskTaker-settleAskMaker-partial-settlements

Valid high, in settleAskTaker/settleAskMaker, if the original offer maker performs a partial final settlement, the existing checks [here](https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/DeliveryPlace.sol#L356-L358) and [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L230-L232) will cause an revert when attempting to complete a full settlement, resulting in their collateral being locked and requiring a rescue from the admin. To note, although examples in the documentation implies settlement in a single click, it is not stated that partial settlements are not allowed, so I believe it is a valid user flow.

Support

FAQs

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