DeFiHardhatFoundry
250,000 USDC
View results
Submission Details
Severity: medium
Invalid

Flawed implementation in `MarketplaceFacet.transferPlot` function leads to unintended pod listing cancellation

Summary

A vulnerability has been identified in the MarketplaceFacet contract that allows any user with partial approval to transfer pods from the owner's plot to cancel the entire PodListing.

Vulnerability Details

The vulnerability exists in the transferPlot function. Specifically, the issue arises when a user with partial approval to transfer pods can also cancel the entire PodListing. The steps to exploit this vulnerability are as follows:

function transferPlot(address sender, address recipient, uint256 fieldId, uint256 index, uint256 start, uint256 end)
external
payable
fundsSafu
noNetFlow
noSupplyChange
nonReentrant
{
require(sender != address(0) && recipient != address(0), "Field: Transfer to/from 0 address.");
1. uint256 transferAmount = validatePlotAndReturnPods(fieldId, sender, index, start, end);
2. if (LibTractor._user() != sender && allowancePods(sender, LibTractor._user(), fieldId) != type(uint256).max) {
2. decrementAllowancePods(sender, LibTractor._user(), fieldId, transferAmount);
}
3. if (s.sys.podListings[fieldId][index] != bytes32(0)) {
3. LibMarket._cancelPodListing(sender, fieldId, index);
}
_transferPlot(sender, recipient, fieldId, index, start, transferAmount);
}
  1. The validatePlotAndReturnPods function determines the transfer amount.

  2. If the user is not the owner and has partial approval, the decrementAllowancePods function decrements the approval amount.

  3. The function checks if there is an active PodListing and cancels it, regardless of the transferred amount of Pods.

LibMarket._cancelPodListing

function _cancelPodListing(address lister, uint256 fieldId, uint256 index) internal {
AppStorage storage s = LibAppStorage.diamondStorage();
require(
s.accts[lister].fields[fieldId].plots[index] > 0,
"Marketplace: Listing not owned by sender."
);
delete s.sys.podListings[fieldId][index];
emit PodListingCancelled(lister, fieldId, index);
}

Exploitation of this issue does not require even a user to be malicious. A user intending to use the system without any malicious intent can unintentionally cancel the entire PodListing due to the flawed implementation.

Exploitation Example

Consider the following scenario involving three participants: Alice (the owner of the PodListing), Bob (a user with partial approval), and Charlie (another user in the marketplace).

  1. Initial Setup:

    • Alice creates a PodListing for 100 pods from her plot.

    • Alice gives Bob approval to transfer 10 pods from her plot.

  2. Exploitation:

    • Bob calls the transferPlot function with start = 0 and end = 10, resulting in a transferAmount = 10.

    • The function validates the plot and decrements Bob's approval by 10 pods.

    • The function checks for an active PodListing and cancels it (however, Bob's intention was not to close the entire PodListing but only to transfer his part of the plot, which Alice gave approval for him to do).

  3. Impact:

    • Despite having approval to transfer only 10 pods, Bob successfully cancels the entire PodListing of remaning 90 pods.

    • Charlie, who was interested in purchasing the listed pods, finds the listing canceled unexpectedly.

    • Alice’s listing was unfairly canceled without her consent, disrupting her plans. Her idea was for users interested in buying pods from her plot to contact her, and for her to give approval to them to do so.

Impact

Users can cancel PodListings without the owner's consent, leading to disruptions in the marketplace and unfair cancellations. This results in a cumbersome user experience for functions like transferPlot and transferPlots, especially when approvals are given from podlisting owners to users to transfer part of their plot. After every transfer, the owner must create a new PodListing, paying gas fees and spending additional time, which is completely unnecessary.

Tools Used

Manual code review

Recommendations

Implement a check that will cancel the PodListing only if the entire amount was transferred.

Updates

Lead Judging Commences

inallhonesty 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.