Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Malicious actors can exploit inconsistent state transitions and collateral handling to manipulate market conditions (`PreMarkets::closeOffer`, `PreMarkets::relistOffer`, `PreMarkets::listOffer`)

4gontuk

Summary

Vulnerability Detail

The PreMarkets contract implements critical functionality for managing offers and stocks in a decentralized marketplace. Three key functions - closeOffer(), relistOffer(), and listOffer() - are responsible for transitioning the state of offers and stocks, as well as handling collateral deposits and refunds. However, these functions contain significant vulnerabilities in their state transition logic and collateral handling mechanisms.

The closeOffer() function updates the offerStatus to Canceled but fails to update the corresponding stockStatus. This inconsistency can lead to confusion and potential exploitation when the stock is later relisted or a new offer is created for it. The relistOffer() function compounds this issue by setting the offerStatus back to Virgin without properly managing the stock's state, potentially creating conflicts if the stock has been associated with another offer in the meantime.

Furthermore, the listOffer() function does not account for the possibility of a stock being previously associated with a different offer. This oversight can result in double-counting of collateral or mismanagement of funds, compromising the financial integrity of the marketplace.

Impact

This issue can severely undermine the integrity and security of the marketplace. Malicious actors could exploit these inconsistencies to manipulate market conditions, potentially leading to financial losses for other participants. The lack of proper state management could result in offers being incorrectly closed, relisted, or created, disrupting the normal functioning of the market.

The collateral mishandling issue in relistOffer() and listOffer() could lead to incorrect calculation and allocation of funds, potentially allowing attackers to game the system for financial gain. This could erode trust in the platform and discourage participation from honest users.

The absence of reentrancy protection exposes the contract to complex attack vectors, where an attacker could potentially drain funds or manipulate market states in ways that are difficult to detect or reverse.

Proof of Concept

  1. Alice creates an offer using listOffer().

  2. Bob takes the offer, partially fulfilling it.

  3. Alice calls closeOffer(), which sets the offerStatus to Canceled but leaves the stockStatus unchanged.

  4. Alice immediately calls relistOffer(), which sets the offerStatus back to Virgin.

  5. Due to the inconsistent state, Bob's partial fulfillment is not properly accounted for, potentially allowing Alice to relist the offer with more collateral than she should have available.

Tools Used

Manual review

Recommended Mitigation Steps

  1. Implement consistent state management:

function closeOffer(address _stock, address _offer) external {
// ... existing checks ...
offerInfo.offerStatus = OfferStatus.Canceled;
+ stockInfo.stockStatus = StockStatus.Canceled;
emit CloseOffer(_offer, _msgSender());
}
​
function relistOffer(address _stock, address _offer) external payable {
// ... existing checks ...
offerInfo.offerStatus = OfferStatus.Virgin;
+ stockInfo.stockStatus = StockStatus.Active;
emit RelistOffer(_offer, _msgSender());
}

  1. Improve collateral handling in relistOffer():

function relistOffer(address _stock, address _offer) external payable {
// ... existing checks ...
if (makerInfo.offerSettleType == OfferSettleType.Protected ||
stockInfo.preOffer == address(0x0)) {
- uint256 depositAmount = OfferLibraries.getRefundAmount(
+ uint256 depositAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
- offerInfo.amount,
+ offerInfo.amount - offerInfo.usedPoints,
offerInfo.points,
offerInfo.usedPoints,
offerInfo.collateralRate
);
// ... handle deposit ...
}
}

  1. Add checks in listOffer() to prevent double-counting of collateral:

function listOffer(address _stock, uint256 _amount, uint256 _collateralRate) external payable {
// ... existing checks ...
+ if (stockInfo.offer != address(0)) {
+ revert StockAlreadyAssociated();
+ }
// ... rest of the function ...
}

  1. Implement reentrancy protection:

+ import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
- contract PreMarkets is PerMarketsStorage, Rescuable, Related, IPerMarkets {
+ contract PreMarkets is PerMarketsStorage, Rescuable, Related, IPerMarkets, ReentrancyGuard {
// ... existing code ...
- function closeOffer(address _stock, address _offer) external {
+ function closeOffer(address _stock, address _offer) external nonReentrant {
// ... function body ...
}
- function relistOffer(address _stock, address _offer) external payable {
+ function relistOffer(address _stock, address _offer) external payable nonReentrant {
// ... function body ...
}
- function listOffer(address _stock, uint256 _amount, uint256 _collateralRate) external payable {
+ function listOffer(address _stock, uint256 _amount, uint256 _collateralRate) external payable nonReentrant {
// ... function body ...
}
}
Updates

Lead Judging Commences

0xnevi Lead Judge
over 1 year ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-Premarkets-stockStatus-update-missing

Valid low severity due to accounting error, although `stock` status is not update appropriately to `Finished`, there will be no exploit possible given relevant checks on the `offer` side.

Support

FAQs

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

Give us feedback!