ComposableSecurity - Malicious collection referrer can brick edition

[sherlock-admin3]

ComposableSecurity

medium

Malicious collection referrer can brick edition

Summary

Each published edition can have a collection referrer that gets their fee on each mint. However, the logic of the referrer can change and it can start to revert ETH transfers leading to DoS of the edition.

Vulnerability Detail

When the edition is published in the _publish function, the collection referrer is set by the publisher.

The referrer will get their fee transferred on each mint in _splitProtocolFee (in fact it does not but that's another issue :)).

However, the referrer can be a contract that changes its logic and starts to revert on ETH transfers, leading to DoS of the edition.

Important: Neither publisher nor edition creator has the ability to remove the malicious referrer.

Impact

The malicious referrer can block minting any new works in their edition.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/TitlesCore.sol#L120-L149

https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/fees/FeeManager.sol#L461-L467

https://github.com/Vectorized/solady/blob/91d5f64b39a4d20a3ce1b5e985103b8ea4dc1cfc/src/utils/SafeTransferLib.sol#L83C1-L91C6

Tool used

Manual Review

Recommendation

Consider adding a function that will allow the creator to remove/change the referrer at later stage.

Additionally:

• Either try to send the fee (function trySafeTransferETH) and if it fails, ignore the fee (it's referrer's responsibility to correctly collect fees) or force transfer the fee (function forceSafeTransferETH).
• Redesign fees so that they can be withdrawn rather than transferred. While this will significantly reduce the cost of minting, it will require securing the funds of fee recipients.

Duplicate of #261

[ComposableSecurityTeam]

Escalate

The issue is valid, as it breaks core contract functionality. Contracts can be updated and in the past we have seen attacks that lead to theft while appearing to have good intentions. A great example is Tornado Cash, where a seemingly harmless proposal went through DAO voting and then was maliciously modified. https://composable-security.com/blog/understanding-the-tornado-cash-governance-attack/

Moreover, the collection referrer might not be controlled by the edition creator so the trusted role rule does not apply here. The malicious referrer can trick the creator to use an address that will have the code deployed before (precalculated CREATE2 address).

[sherlock-admin3]

Escalate

The issue is valid, as it breaks core contract functionality. Contracts can be updated and in the past we have seen attacks that lead to theft while appearing to have good intentions. A great example is Tornado Cash, where a seemingly harmless proposal went through DAO voting and then was maliciously modified. https://composable-security.com/blog/understanding-the-tornado-cash-governance-attack/

Moreover, the collection referrer might not be controlled by the edition creator so the trusted role rule does not apply here. The malicious referrer can trick the creator to use an address that will have the code deployed before (precalculated CREATE2 address).

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[Hash01011122]

Any thoughts @pqseags??

[pqseags]

I believe @ccashwell disputed this, so I'll let him chime in here if he can, but my read is that this could be valid and worth a fix.

[ccashwell]

I don't believe it's valid. There's no incentive to do it as a referrer, and giving the Edition owner the ability to remove/reset referrer address is worse because it enables the owner to effectively steal future referral benefits.

[ComposableSecurityTeam]

This issue is of medium severity for the following reasons:

• The issue is technically feasible.
• A malicious referrer can generate an address without the code (using CREATE2) that appears as an externally owned account (EOA) and provide it to the publisher, concealing the potential attack. It can use such approach for all editions.
• A malicious referrer might seek to disrupt editions for various reasons, such as personal conflicts with the creator and a desire to cause harm, particularly when the associated fee is minimal.

To mitigate the risk of a malicious owner altering the fee receiver address, a potential solution is to deploy a predefined receiver contract with fees that are withdrawable only by the referrer. This approach ensures the project remains uninfluenced by the referrer, and the edition creator is unable to modify the referrer address.

[ccashwell]

1. Conceptual technical feasibility isn't the criteria for validity. Impact is important.

2. The only way to refer a collection is to convince a creator to generate a new collection through a referrer-owned interface. In that context, if the malicious user intended to cause harm or disruption, it's more efficient to simply scam the creator out of his money. There's no mechanism for a creator to specify a collection referrer otherwise.

3. I totally get that you want to be compensated for this finding, but let's be real... this is NOT an actionable finding and you're reaching very far to try to make it seem valid.

[ComposableSecurityTeam]

I disagree with statement "The only way to refer a collection is to convince a creator to generate a new collection through a referrer-owned interface.". If I understand you correctly, you mean that the creator (actually the publisher) would be able to see that the referrer address is a malicious contract.

The fact is the referrer address might be a EOA-looking address which in real is precomputed address of a contract to be deployed via CREATE2. It only needs to have the logic changed for the fallback function. Please note that the referrer role is not listed as trusted, so attack vectors from their perspective are in scope.

What is more, the referrer could use the same address for multiple editions and works and later deploy the malicious contract and manage which work shall revert (allowing for ransoms).

The impact is medium due to the following rule:
Breaks core contract functionality, rendering the contract useless or leading to loss of funds.

You always have the option to not fix finding and accept the risk, but it does not change the fact that the problem exists.

[ccashwell]

What you're describing is social engineering, not a contract vulnerability, and the proposed solution is to essentially allow the content creator to override the referral context.

In any case, what you're assuming isn't what I was suggesting as the limiting factor at all. What I'm saying is that the venue for creating content (the TITLES app, or an alternate client with sufficient functionality to enable content creation) identifies the referrer as the source of traffic and rewards them for the effort over time.

The only way to be tagged as the collection referrer is to actually drive the creation through the standard venue, which would mean they need to advertise broadly (or social engineer a specific target user) a ref link with a malicious address. This is an inherently self-limiting approach that could be described at the very most extreme as griefing.

(There's another approach that is theoretically possible, which is to convince the creator to call the contract directly with a maliciously crafted publish payload, but it's at least as challenging as the above described scenario.)

Nothing about this issue "breaks functionality" or makes any portion of any contract in the system "useless" as per the rule you've cited.

[ComposableSecurityTeam]

"What you're describing is social engineering, not a contract vulnerability"

This statement is false because the victim (publisher) does not simply execute a privileged operation with attacker's profit (like passing the ownership to the attacker). The victim executes a typical operation - gets a valid address from the referrer as it normally would and assigns it to the work. The victim has no way to detect possible attack as the malicious referrer provides a typical EOA address. It is more like a role in the project becomes malicious - non-trusted role becoming malicious.

"and the proposed solution is to essentially allow the content creator to override the referral context."

The proposed solution was updated to be permissionless and does not lead to override. It allows you to use the pull over push mechanism on the contract that you will fully control. Although the referrer will still be a smart contract, this will not have the consequences it currently has.

"The only way to be tagged as the collection referrer is to actually drive the creation through the standard venue, which would mean they need to advertise broadly (or social engineer a specific target user) a ref link with a malicious address. This is an inherently self-limiting approach that could be described at the very most extreme as griefing."

This statement is partially true, but it does not take into account the significant monetization opportunity for the malicious referrer. It is not difficult to become a referrer and anyone can become one. The malicious referrers have incentives for such attacks. They can wait until all works referred by him get value and then demand a ransom from the creators.

"Nothing about this issue "breaks functionality" or makes any portion of any contract in the system "useless" as per the rule you've cited."

It breaks the minting of the works which IS a core functionality.

What you proposed in brackets is actually a social engineering - the malicious payload is detectable from the beginning and in the attack vector we detected the address used by the referrer cannot be detected as malicious until they attack the protocol.

You correctly point out various limitations and we are aware of them and agree with some of them. That's why this issue is MEDIUM and not HIGH.

Maybe it's worth getting an outside perspective here. @WangSecurity @Evert0x

[WangSecurity]

Agree with the escalation, even though it's pure griefing and the referrers lose fees themselves, the protocol also loses fee. Planning to accept and duplicate with #261

[Evert0x]

Result:
Medium
Duplicate of #261

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• ComposableSecurityTeam: accepted