ubl4nk - TitlesCore#_publish is not returning back the excess funds to Edition/Works creator

[sherlock-admin3]

ubl4nk

medium

TitlesCore#_publish is not returning back the excess funds to Edition/Works creator

Summary

TitlesCore#_publish is not returning back the excess funds to Edition/Works creator which may lead to loss of funds for Edition/Work creator.

Vulnerability Detail

There is no functionality to return the excess funds to the Edition/Work creator in _publish function and due to it, 2 scenarios might happen which will lead to loss of funds for creator.

Scenario_1:

• Alice is a work creator
• Alice calls publish to publish a new work (or calls createEdition to create new Edition and publish a new work)
• Alice reaches to this line:

function _publish(Edition edition_, WorkPayload memory work_, address referrer_) // address of Edition, EditionPayload.work, address of referrer
        internal
        returns (uint256 tokenId)
    {
        // ... SKIP .. //

@>        feeManager.collectCreationFee{value: msg.value}(edition_, tokenId, msg.sender);

It collects a protocolCreationFee, this means Alice should pay a protocolCreationFee for creation of new works.

• If Alice has set a higher value than protocolCreationFee, Alice will lose the excess Ethers/Funds who paid for protocolCreationFee (this is because the function is not returning the extra funds to Alice)

Scenario_2:

• Assuming now the protocolCreationFee is 0.2 ETH
• Admin decides to decrease the creation fee.
• Alice see the creation fee is now 0.2 ETH and sets 0.2 ETH to its transaction and calls publish to publish a new work
• Admin calls setProtocolFees and decreases the protocolCreationFee to 0.1 ETH
• Admin transaction will be executed
• Alice transaction will be executed
• Alice has lost 0.1 ETH (because the creation fee changed to 0.1 but Alice paid 0.2 ETH and the publish is not returning the excess funds)

Impact

Edition/Works creators may experience a loss while creating new works.

Code Snippet

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

https://github.com/sherlock-audit/2024-04-titles/blob/c9d16782a7d3c15c7a759f22c9e0552d5e777ed7/wallflower-contract-v2/src/fees/FeeManager.sol#L166-L174

https://github.com/sherlock-audit/2024-04-titles/blob/c9d16782a7d3c15c7a759f22c9e0552d5e777ed7/wallflower-contract-v2/src/fees/FeeManager.sol#L298-L316

Tool used

Manual Review

Recommendation

Consider returning the extra funds in publish.

[Alireza-Razavi]

escalate
With much respects to judge decisions, but i think this is a valid issue and the sponsor itself confirmed the issue in Discord.
I have provided 2 scenarios which might happen and besides those, this is a lack in the protocol.

[sherlock-admin3]

escalate
With much respects to judge decisions, but i think this is a valid issue and the sponsor itself confirmed the issue in Discord.
I have provided 2 scenarios which might happen and besides those, this is a lack in the protocol.

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.

[Evert0x]

Escalation should be rejected. This is a design recommendation for the protocol team, there is no security issue.

[Alireza-Razavi]

@Evert0x @WangSecurity
Please Read the scenario_2.
This is an edge case issue that might happen, this is also previously confirmed by the sponsor.

[WangSecurity]

I believe with admins of the protocol being trusted, it's not a valid scenario. Specifically, this aspect of the trsuted admin rule:

An admin action can break certain assumptions about the functioning of the code. Example: Pausing a collateral causes some users to be unfairly liquidated or any other action causing loss of funds. This is not considered a valid issue.

Hence, decision remains the same, reject the escalation and leave the issue as it is.

[Evert0x]

Result:
Invalid
Unique

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• alireza-razavi: rejected