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

Inconsistent Settlement Handling Will Lead to Incorrect Offer States and Potential Financial Losses (`PreMarkets::settledAskOffer `and `PreMarkets::settleAskTaker`)

4gontuk

Summary

Vulnerability Detail

The PreMarkets contract implements two critical functions for settling offers: settledAskOffer() and settleAskTaker(). These functions are designed to handle the settlement process for ask offers and bid takers respectively. However, there is a significant inconsistency in how these functions update the settlement state, which can lead to incorrect offer states and potential financial discrepancies.

In the settledAskOffer() function, the settledPoints and settledPointTokenAmount are directly set to the input values:

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;
// ...
}

Conversely, in the settleAskTaker() function, these same values are incremented:

function settleAskTaker(
address _offer,
address _stock,
uint256 _settledPoints,
uint256 _settledPointTokenAmount
) external onlyDeliveryPlace(tadleFactory, _msgSender()) {
// ...
offerInfo.settledPoints += _settledPoints;
offerInfo.settledPointTokenAmount += _settledPointTokenAmount;
// ...
}

This inconsistency can lead to incorrect settlement states. For instance, if settledAskOffer() is called after settleAskTaker(), it will overwrite the accumulated values, potentially leading to a loss of settlement information.

Furthermore, settledAskOffer() sets the offerStatus to Settled, while settleAskTaker() does not update this status at all. This can result in offers being marked as settled prematurely or remaining unsettled when they should be marked as settled.

Impact

The inconsistent handling of settlement states can lead to incorrect financial accounting and potential discrepancies in the offer and stock states. This can result in financial losses for users and undermine the integrity of the contract's settlement process. Specifically, offers may appear unsettled when they are partially settled, leading to incorrect balances and potential exploitation.

Proof of Concept

  1. Alice calls settleAskTaker() for an offer, incrementing settledPoints by 100 and settledPointTokenAmount by 1000.

  2. Bob calls settleAskTaker() for the same offer, further incrementing settledPoints by 50 and settledPointTokenAmount by 500.

  3. Charlie calls settledAskOffer() for the same offer with _settledPoints as 75 and _settledPointTokenAmount as 750.

  4. The final state of the offer now shows settledPoints as 75 and settledPointTokenAmount as 750, effectively erasing the settlements made by Alice and Bob.

Tools Used

Manual review

Recommendation

To address this issue, both functions should handle the settlement state consistently. Here's a proposed fix:

function settledAskOffer(
address _offer,
uint256 _settledPoints,
uint256 _settledPointTokenAmount
) external onlyDeliveryPlace(tadleFactory, _msgSender()) {
OfferInfo storage offerInfo = offerInfoMap[_offer];
- offerInfo.settledPoints = _settledPoints;
- offerInfo.settledPointTokenAmount = _settledPointTokenAmount;
+ offerInfo.settledPoints += _settledPoints;
+ offerInfo.settledPointTokenAmount += _settledPointTokenAmount;
offerInfo.offerStatus = OfferStatus.Settled;
// ...
}
​
function settleAskTaker(
address _offer,
address _stock,
uint256 _settledPoints,
uint256 _settledPointTokenAmount
) external onlyDeliveryPlace(tadleFactory, _msgSender()) {
// ...
offerInfo.settledPoints += _settledPoints;
offerInfo.settledPointTokenAmount += _settledPointTokenAmount;
+ offerInfo.offerStatus = OfferStatus.Settled;
// ...
}

Additionally, consider implementing a check to ensure that the total settled points and amounts do not exceed the original offer amounts.

Updates

Lead Judging Commences

0xnevi Lead Judge
about 1 year ago
0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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