Submission Details
Severity:
transferFrom function is being called with an arbitrary from address, which could potentially lead to security vulnerabilities.
Summary : The issue here is that the transferFrom function is being called with an arbitrary from address, which could potentially lead to security vulnerabilities.
Vulnerability Details : The issue here is that the transferFrom function is being called with an arbitrary from address, which could potentially lead to security vulnerabilities.
In Swan.sol, the function transferFrom has this vulnerability.
function transferRoyalties(AssetListing storage asset) internal {
// calculate fees
uint256 buyerFee = (asset.price * asset.royaltyFee) / 100;
uint256 driaFee = (buyerFee * getCurrentMarketParameters().platformFee) / 100;
// first, Swan receives the entire fee from seller
// this allows only one approval from the seller's side
token.transferFrom(asset.seller, address(this), buyerFee);
///////rest of code
}
Impact : In swan.purchase function, as given below, the line SwanAsset(_asset).transferFrom(listing.seller, address(this), 1); is transferring an asset from listing.seller to the contract itself (address(this)).
However, the transferFrom function does not check if the from address (listing.seller) is actually the owner of the asset being transferred. This means that if an attacker can manipulate the listing.seller variable to point to an address that is not the owner of the asset, they could potentially steal the asset.
/// @notice Executes the purchase of a listing for a buyer for the given asset.
/// @dev Must be called by the buyer of the given asset.
function purchase(address _asset) external {
AssetListing storage listing = listings[_asset];
// asset must be listed to be purchased
if (listing.status != AssetStatus.Listed) {
revert InvalidStatus(listing.status, AssetStatus.Listed);
}
// can only the buyer can purchase the asset
if (listing.buyer != msg.sender) {
revert Unauthorized(msg.sender);
}
// update asset status to be sold
listing.status = AssetStatus.Sold;
// transfer asset from seller to Swan, and then from Swan to buyer
// this ensure that only approval to Swan is enough for the sellers
SwanAsset(_asset).transferFrom(listing.seller, address(this), 1);
SwanAsset(_asset).transferFrom(address(this), listing.buyer, 1);
// transfer money
token.transferFrom(listing.buyer, address(this), listing.price);
token.transfer(listing.seller, listing.price);
emit AssetSold(listing.seller, msg.sender, _asset, listing.price);
}
Tools Used : Slither, VS code
Recommendations : To fix this, you should add a check to ensure that the from address is indeed the owner of the asset being transferred. This can be done by calling the ownerOf function on the SwanAsset contract, like this:
require(SwanAsset(_asset).ownerOf(1) == listing.seller, "Invalid owner");
SwanAsset(_asset).transferFrom(listing.seller, address(this), 1);
Prize pool breakdown
Total prize
21,000 USDC
nSLOC
1,03420 USDC / LOC
20,000 USDC
1,000 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.