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

Arithmetic Overflow in Swan Protocol Fee Calculations

foxb868

Summary

Swan.sol allows fee bypass through price overflow in listing and royalty calculations.

Vulnerability Details

https://github.com/Cyfrin/2024-10-swan-dria/blob/c3f6f027ed51dd31f60b224506de2bc847243eb7/contracts/swan/Swan.sol#L157-L191

function list(string calldata _name, string calldata _symbol, bytes calldata _desc, uint256 _price, address _buyer)
external
{
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);
}
​
// @bug detected: No validation on _price parameter allowing overflow in fee calculations
// This can lead to bypass of fee payments when price > uint256.max/2
address asset = address(swanAssetFactory.deploy(_name, _symbol, _desc, msg.sender));
listings[asset] = AssetListing({
createdAt: block.timestamp,
royaltyFee: buyer.royaltyFee(),
price: _price, // @bug detected: Unchecked price storage
seller: msg.sender,
status: AssetStatus.Listed,
buyer: _buyer,
round: round
});
​
// @bug detected: Potential overflow in transferRoyalties() when calculating:
// buyerFee = price * royaltyFee / 100
transferRoyalties(listings[asset]);
​
emit AssetListed(msg.sender, asset, _price);
}

The issue is the list() function where there is no check for price overflow when:

  1. Storing the price in the AssetListing struct

  2. Calculating royalty fees: uint256 buyerFee = (asset.price * asset.royaltyFee) / 100;

Impact:

  • If price is very large, the multiplication price * royaltyFee could overflow

  • This could lead to incorrect fee calculations and token transfers

  • In extreme cases, could allow bypassing fee payments entirely

Proof of Concept:

  1. Call list() with a price > uint256.max/2

  2. The royalty calculation will overflow

  3. Actual fees paid will be much lower than intended

// Test scenario
1. Set royaltyFee = 10 (10%)
2. List asset with price = 2^256-1
3. Expected fee = (2^256-1 * 10) / 100
4. Actual result: Overflow occurs, fee calculation becomes very small
5. Result: Protocol receives minimal fees instead of intended 10%
​
// Example values:
price = 2^256-1
royaltyFee = 10
Actual fee paid ≈ 0 (due to overflow)
Expected fee = price * 10 / 100

Why It's Dangerous:

  1. Direct financial impact on protocol revenue

  2. Affects every listing with large prices

  3. Can be exploited systematically

  4. Undermines core protocol economics

  5. Connected to critical token transfer logic

Impact

  1. Malicious users can manipulate fee calculations by using large prices that cause arithmetic overflow

  2. The protocol and buyer agents could receive much lower fees than intended

Tools Used

Manual Review

Recommendations

Ensures fee calculations cannot overflow and maintains protocol economics integrity.

Updates

Lead Judging Commences

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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