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 {
(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) {
return _computePhase(marketParams[marketParameterIdx], block.timestamp - createdAt);
} else {
uint256 idx = marketParameterIdx;
(uint256 round,,) = _computePhase(marketParams[idx], marketParams[idx + 1].timestamp - createdAt);
idx++;
while (idx < marketParams.length - 1) {
(uint256 innerRound,,) =
_computePhase(marketParams[idx], marketParams[idx + 1].timestamp - marketParams[idx].timestamp);
round += innerRound + 1;
idx++;
}
(uint256 lastRound, Phase phase, uint256 timeRemaining) =
@> _computePhase(marketParams[idx], block.timestamp - marketParams[idx].timestamp);
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
:
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 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;
Swan swan;
SwanAssetFactory swanAssetFactory;
LLMOracleCoordinator coordinator;
LLMOracleRegistry registry;
BuyerAgentFactory buyerAgentFactory;
ERC20Mock token;
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
});
LLMOracleTaskParameters oracleParameters =
LLMOracleTaskParameters({difficulty: 1, numGenerations: 1, numValidations: 1});
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);
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)
{
vm.warp(_createTimeStamp);
vm.prank(buyerAgentOwner);
buyerAgent = swan.createBuyer("BuyerAgent", "Buyooor", _royaltyFee, _amountPerRound);
}
function generatorAndValidatorStake() public {
for (uint256 i = 0; i < generators.length; i++) {
vm.startPrank(generators[i]);
token.approve(address(registry), generatorStakeAmount);
registry.register(LLMOracleKind.Generator);
}
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
:
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;
uint256 amountPerRound = 1e18;
uint256 createTimeStamp = 1000;
function setUp() public override {
super.setUp();
buyerAgent = createBuyerAgent(royaltyFee, amountPerRound, createTimeStamp);
}
function test_setMarketParameterAtSellPhase_wouldMakeListingExpired() public {
(uint256 round,,) = buyerAgent.getRoundPhase();
assertEq(round, 0);
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);
uint256 timeStampWhenSetNewParams = createTimeStamp + sellInterval - 10;
vm.warp(timeStampWhenSetNewParams);
vm.startPrank(deployer);
SwanMarketParameters memory params = SwanMarketParameters({
withdrawInterval: withdrawInterval,
sellInterval: sellInterval,
buyInterval: buyInterval,
platformFee: platformFee,
maxAssetCount: maxAssetCount,
timestamp: timestamp
});
swan.setMarketParameters(params);
vm.stopPrank();
(round,,) = buyerAgent.getRoundPhase();
assertEq(round, 1);
vm.warp(timeStampWhenSetNewParams + sellInterval + 10);
vm.startPrank(buyerAgentOwner);
buyerAgent.oraclePurchaseRequest("0x", "0x");
uint256 taskIdRound0 = buyerAgent.oraclePurchaseRequests(0);
assertEq(taskIdRound0, 0);
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:
free relist on next round for affected creator (recommended)
refund the creator for the royalty fee paid
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);
}
}