Summary

Critical reentrancy vulnerabilities in Swan's core trading functions allow double spending of assets and manipulation of listing states due to state updates occurring after external calls.

Vulnerability Details

Looking at the Swan.sol contract, there are several potential reentrancy vectors.

In the purchase() function: https://github.com/Cyfrin/2024-10-swan-dria/blob/c3f6f027ed51dd31f60b224506de2bc847243eb7/contracts/swan/Swan.sol#L276-L290

A malicious buyer could:

1. Call purchase()

2. During the first transferFrom, reenter the contract

3. The listing status is still Listed since it hasn't been updated yet

4. This allows buying the same asset multiple times

In the list function: https://github.com/Cyfrin/2024-10-swan-dria/blob/c3f6f027ed51dd31f60b224506de2bc847243eb7/contracts/swan/Swan.sol#L157-L182

In the relist() function: https://github.com/Cyfrin/2024-10-swan-dria/blob/c3f6f027ed51dd31f60b224506de2bc847243eb7/contracts/swan/Swan.sol#L197-L246

These vulnerabilities allow reentrancy attacks because they violate the Checks-Effects-Interactions pattern. The key issue is that external calls (transferFrom/transfer) occur before state updates.

Root Cause:

• The contract performs external calls (transferFrom/transfer) before updating critical state variables

• This violates the Checks-Effects-Interactions pattern

• The listing status remains "Listed" during external calls, allowing reentrant purchases

Why It's Dangerous:

• Breaks core invariants around asset ownership and listing status

• Allows economic attacks through double spending

• Impacts all users interacting with listings

• Connected to core purchase flow affecting all asset transfers

• No protection against reentrancy in place

The vulnerability exists in the main trading functions (purchase, list, relist) which are central to the protocol's functionality. This makes it particularly severe as it affects the core economic mechanisms of the system.

Impact

1. Assets can be purchased multiple times before status updates

2. Royalty payments can be manipulated through reentrancy

3. Listing states can be exploited during reentrant calls

4. Potential loss of funds through double-spending attacks

The attack vectors exist because a malicious contract could reenter during the external calls and execute additional transactions while the state is still unchanged. This allows exploitation of the interim state before updates are finalized.

Tools Used

Manual code review

Recommendations

We can apply a similar pattern to list() and relist() - update state before making external calls. Consider adding reentrancy guards to critical functions.