Dria

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

Premature Round Transition in `BuyerAgent` Due to Market Parameter Changes Would Make Financial and Opportunity Lost on Creator

Summary

A vulnerability in the BuyerAgent contract causes premature round transitions when market parameters are updated. This issue affects creators listing assets during the Phase.sell phase, leading to financial and opportunity losses. The round transitions from sell -> newround -> sell -> buy -> withdraw instead of the intended sell -> buy -> withdraw, causing creators to lose their right to be picked by agents in the Phase.buy phase despite paying the royalty/listing fee.

Vulnerability Details

BuyerAgent.sol#L189-L195

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());
}

when buyerAgent is on buy phase, it would eventually call oraclePurchaseRequest where the function would check if its on Phase.Buy and it would return current round phase.
this round is used to call LLMOracleCoordinator::request via Swan contract.

however, when there are new market parameter sets, the round is prematurely ends and it would begins new round for all current buyerAgent

SwanManager.sol#L80-L84

function setMarketParameters(SwanMarketParameters memory _marketParameters) external onlyOwner {
require(_marketParameters.platformFee <= 100, "Platform fee cannot exceed 100%");
_marketParameters.timestamp = block.timestamp;
marketParameters.push(_marketParameters);
}

this is because the _marketParameters.timestamp is updated at current block.timestamp when calling setMarketParameters.
if we check _checkRoundPhase used to calculate the round, it would then call getRoundPhase:

BuyerAgent.sol#L334-L374

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);
}
}

on the line I marked block.timestamp - marketParams[idx].timestamp would cause the changing of round when calling setMarketParameters.

the unintended cause is there would be a potential issue for creator that listing for buyerAgent whose at the time of market parameter change is still on Phase.sell.
the issue is the creator would pay for royalty fee to buyerAgent for list/relist asset but the round is never completed in order of sell -> buy -> withdraw instead sell -> newround -> sell -> buy -> withdraw.
so at the round this market parameter changed, the creator would lost their right to be picked by agent in the Phase.buy even though they already paid the royalty/listing fee.

Proof of Code:

add this file to test folder:

Base.t.sol:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
import {Test} from "../lib/forge-std/src/Test.sol";
import {console2} from "../lib/forge-std/src/console2.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol";
import {Swan} from "../contracts/swan/Swan.sol";
import {SwanAssetFactory, SwanAsset} from "../contracts/swan/SwanAsset.sol";
import {SwanManager, SwanMarketParameters} from "../contracts/swan/SwanManager.sol";
import {BuyerAgentFactory, BuyerAgent} from "../contracts/swan/BuyerAgent.sol";
import {LLMOracleTaskParameters} from "../contracts/llm/LLMOracleTask.sol";
import {LLMOracleCoordinator} from "../contracts/llm/LLMOracleCoordinator.sol";
import {LLMOracleRegistry, LLMOracleKind} from "../contracts/llm/LLMOracleRegistry.sol";
import {ERC20Mock} from "./ERC20Mock.sol";
contract Base is Test {
// address
address deployer = makeAddr("deployer");
address buyerAgentOwner = makeAddr("buyerAgentOwner");
address creator = makeAddr("creator");
address generator1 = makeAddr("generator1");
address generator2 = makeAddr("generator2");
address[] generators;
address validator1 = makeAddr("validator1");
address validator2 = makeAddr("validator2");
address validator3 = makeAddr("validator3");
address[] validators;
// contracts
Swan swan;
SwanAssetFactory swanAssetFactory;
LLMOracleCoordinator coordinator;
LLMOracleRegistry registry;
BuyerAgentFactory buyerAgentFactory;
ERC20Mock token;
// market parameters
uint256 withdrawInterval = 30 minutes;
uint256 sellInterval = 60 minutes;
uint256 buyInterval = 10 minutes;
uint256 platformFee = 1;
uint256 maxAssetCount = 5;
uint256 timestamp = 0;
SwanMarketParameters marketParameters = SwanMarketParameters({
withdrawInterval: withdrawInterval,
sellInterval: sellInterval,
buyInterval: buyInterval,
platformFee: platformFee,
maxAssetCount: maxAssetCount,
timestamp: timestamp
});
// oracle parameters
LLMOracleTaskParameters oracleParameters =
LLMOracleTaskParameters({difficulty: 1, numGenerations: 1, numValidations: 1});
// stake parameters
uint256 generatorStakeAmount = 0.1e18;
uint256 validatorStakeAmount = 0.1e18;
uint256 stakePlatformFee = 0.001e18;
uint256 generationFee = 0.002e18;
uint256 validationFee = 0.003e18;
function setUp() public virtual {
vm.startPrank(deployer);
token = new ERC20Mock("Mock", "MOCK");
buyerAgentFactory = new BuyerAgentFactory();
swanAssetFactory = new SwanAssetFactory();
LLMOracleRegistry registryImpl = new LLMOracleRegistry();
ERC1967Proxy registryProxy = new ERC1967Proxy(
address(registryImpl),
abi.encodeWithSelector(
LLMOracleRegistry.initialize.selector, generatorStakeAmount, validatorStakeAmount, address(token)
)
);
registry = LLMOracleRegistry(address(registryProxy));
LLMOracleCoordinator coordinatorImpl = new LLMOracleCoordinator();
ERC1967Proxy coordinatorProxy = new ERC1967Proxy(
address(coordinatorImpl),
abi.encodeWithSelector(
LLMOracleCoordinator.initialize.selector,
address(registry),
address(token),
stakePlatformFee,
generationFee,
validationFee
)
);
coordinator = LLMOracleCoordinator(address(coordinatorProxy));
Swan swanImpl = new Swan();
ERC1967Proxy swanProxy = new ERC1967Proxy(
address(swanImpl),
abi.encodeWithSelector(
Swan.initialize.selector,
marketParameters,
oracleParameters,
address(coordinator),
address(token),
address(buyerAgentFactory),
address(swanAssetFactory)
)
);
swan = Swan(address(swanProxy));
assertEq(swan.getMarketParameters().length, 1);
vm.stopPrank();
generators = new address[](2);
generators[0] = generator1;
generators[1] = generator2;
validators = new address[](3);
validators[0] = validator1;
validators[1] = validator2;
validators[2] = validator3;
vm.stopPrank();
token.mint(buyerAgentOwner, 10e18);
token.mint(creator, 10e18);
// give token to generators, validators
token.mint(generator1, generatorStakeAmount);
token.mint(generator2, generatorStakeAmount);
token.mint(validator1, validatorStakeAmount);
token.mint(validator2, validatorStakeAmount);
token.mint(validator3, validatorStakeAmount);
}
function createBuyerAgent(uint96 _royaltyFee, uint256 _amountPerRound, uint256 _createTimeStamp)
public
returns (BuyerAgent buyerAgent)
{
// create buyer agent
vm.warp(_createTimeStamp);
vm.prank(buyerAgentOwner);
buyerAgent = swan.createBuyer("BuyerAgent", "Buyooor", _royaltyFee, _amountPerRound);
}
function generatorAndValidatorStake() public {
// generator stake
for (uint256 i = 0; i < generators.length; i++) {
vm.startPrank(generators[i]);
token.approve(address(registry), generatorStakeAmount);
registry.register(LLMOracleKind.Generator);
}
//validator stake
for (uint256 i = 0; i < validators.length; i++) {
vm.startPrank(validators[i]);
token.approve(address(registry), validatorStakeAmount);
registry.register(LLMOracleKind.Validator);
}
}
function createListing_Amount_For_WithPrice(address _creator, uint256 _amount, address _buyerAgent, uint256 _price)
public
returns (address[] memory assets)
{
vm.startPrank(_creator);
swan.token().approve(address(swan), _price * _amount);
bytes memory desc = "description";
for (uint256 i = 0; i < _amount; i++) {
swan.list(string(abi.encodePacked("test ", uint2str(i))), "TEST", desc, _price, _buyerAgent);
}
(uint256 round,,) = BuyerAgent(_buyerAgent).getRoundPhase();
assets = swan.getListedAssets(_buyerAgent, round);
vm.stopPrank();
}
function uint2str(uint256 _i) internal pure returns (string memory) {
if (_i == 0) {
return "0";
}
uint256 j = _i;
uint256 length;
while (j != 0) {
length++;
j /= 10;
}
bytes memory bstr = new bytes(length);
uint256 k = length;
while (_i != 0) {
k = k - 1;
uint8 temp = (48 + uint8(_i - _i / 10 * 10));
bytes1 b1 = bytes1(temp);
bstr[k] = b1;
_i /= 10;
}
return string(bstr);
}
function test_createListing() public {
uint256 amount = 5;
BuyerAgent buyerAgent = createBuyerAgent(10, 1e18, 1000);
address[] memory assets = createListing_Amount_For_WithPrice(creator, amount, address(buyerAgent), 100);
assertEq(swan.getListedAssets(address(buyerAgent), 0).length, amount);
for (uint256 i = 0; i < assets.length; i++) {
console2.log("asset: ", assets[i]);
}
}
function test_stake() public {
generatorAndValidatorStake();
}
}

Swan.t.sol:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
import {console2} from "../lib/forge-std/src/console2.sol";
import {Base} from "./Base.t.sol";
import {BuyerAgent} from "../contracts/swan/BuyerAgent.sol";
import {SwanMarketParameters} from "../contracts/swan/SwanManager.sol";
import {LLMOracleTaskParameters} from "../contracts/llm/LLMOracleTask.sol";
contract SwanTest is Base {
BuyerAgent buyerAgent;
uint96 royaltyFee = 10; // 10%
uint256 amountPerRound = 1e18;
// for the sake of testing, we set the create time stamp to 1000
uint256 createTimeStamp = 1000;
function setUp() public override {
super.setUp();
buyerAgent = createBuyerAgent(royaltyFee, amountPerRound, createTimeStamp);
}
function test_setMarketParameterAtSellPhase_wouldMakeListingExpired() public {
// check if round is 0 (first round)
(uint256 round,,) = buyerAgent.getRoundPhase();
assertEq(round, 0);
// sell phase
// creator create listing for buyerAgent
uint256 assetAmount = 3;
uint256 assetPrice = 0.1e18;
uint256 balanceOfCreatorBefore = token.balanceOf(creator);
createListing_Amount_For_WithPrice(creator, assetAmount, address(buyerAgent), assetPrice);
uint256 balanceOfCreatorAfter = token.balanceOf(creator);
uint256 feePaidForListing3Asset = balanceOfCreatorBefore - balanceOfCreatorAfter;
console2.log("Creator paid ", feePaidForListing3Asset, " for listing assets at round", round);
// time warp but still inside sell phase
uint256 timeStampWhenSetNewParams = createTimeStamp + sellInterval - 10;
vm.warp(timeStampWhenSetNewParams);
// set market parameter, this would make the listing expired
vm.startPrank(deployer);
// for simplicity we use same parameter as the default
SwanMarketParameters memory params = SwanMarketParameters({
withdrawInterval: withdrawInterval,
sellInterval: sellInterval,
buyInterval: buyInterval,
platformFee: platformFee,
maxAssetCount: maxAssetCount,
timestamp: timestamp
});
swan.setMarketParameters(params);
vm.stopPrank();
// the round should be 1 now
(round,,) = buyerAgent.getRoundPhase();
assertEq(round, 1);
// warp to buy phase
vm.warp(timeStampWhenSetNewParams + sellInterval + 10);
// buyerAgent try buy the asset from round 0, they dont know if the round is expired
vm.startPrank(buyerAgentOwner);
buyerAgent.oraclePurchaseRequest("0x", "0x"); // using mock data
// round 0 listing is not requested to oracle, thus return 0
uint256 taskIdRound0 = buyerAgent.oraclePurchaseRequests(0);
assertEq(taskIdRound0, 0);
// round 1 listing is requested to oracle, should return taskId 1
uint256 taskIdRound1 = buyerAgent.oraclePurchaseRequests(1);
assertEq(taskIdRound1, 1);
}
}

then run forge test --mt test_setMarketParameterAtSellPhase_wouldMakeListingExpired

we would know if above scenario is happened because the taskId for round 0 is 0 (not requested)
so there no way for the subsequent round the listed asset at round 0 have potential to get picked by oracle
the only way to get picked by the oracle is to relist the asset again

Impact

financial and opportunity loss for creator

Tools Used

foundry

Recommendations

there are many approach for resolving this issue. the main idea is to capture the value of timeElapsed for last round before the market parameter changed timeElapsedBeforeParamsChanged.
if timeElapsedBeforeParamsChanged <= sellIntervalOnLastMarketParameter then we can go to one of the solution:

  1. free relist on next round for affected creator (recommended)

  2. refund the creator for the royalty fee paid

diff --git a/contracts/swan/BuyerAgent.sol b/contracts/swan/BuyerAgent.sol
index 6660a67..4ccc9b0 100644
--- a/contracts/swan/BuyerAgent.sol
+++ b/contracts/swan/BuyerAgent.sol
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);
+ (uint256 round,,uint256 timeUntilNextPhase) = _computePhase(marketParams[idx], marketParams[idx + 1].timestamp - createdAt);
+ // check if we skip buy phase in this round
+ if (timeUntilNextPhase <= marketParams[idx].sellInterval) {
+ // logic to add creator to mapping of eligible free relist on the next round
+ // or logic to refund the creator
+ }
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);
}
}
Updates

Lead Judging Commences

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

Support

FAQs

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