Open sherlock-admin4 opened 3 months ago
While this is unintuitive, we do not intend to update the fee splits when ownership of a work is changed after publishing
Escalate
Should be low/info as this is just un-intuitive behaviour of the protocol and does not represent a real threat / loss of funds.
The team does not intend to update the behaviour confirming that this is intended.
The problem arises when users transfer a work to another user while also believing it will transfer the ownership of the fees. This is a user mistake / misuse of the protocol and not an attack / exploit of a vulnerability.
Escalate
Should be low/info as this is just un-intuitive behaviour of the protocol and does not represent a real threat / loss of funds.
The team does not intend to update the behaviour confirming that this is intended.
The problem arises when users transfer a work to another user while also believing it will transfer the ownership of the fees. This is a user mistake / misuse of the protocol and not an attack / exploit of a vulnerability.
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.
@pqseags @cducrest can you please elaborate on how this issue just un-intuitive behavior of the protocol? Also where was it mentioned at the time of contest that protocol does not intend to update the fee splits when ownership of a work is changed after publishing
It was not documented, but there isn't any logic in the code that makes an attempt to modify the fee route. In our minds, changing the creator on a work is more for the sake of changing who has admin responsibility. As this wasn't documented clearly in the read-me, I'll leave it to the judges to determine severity level given the context.
It is not possible to change the fee split whether before or after ownership of a work is changed. To me, this is an optional feature request. It is not like it was possible to change the fee / royalty target before the work was transferred to a new creator and it becomes no longer possible. It is never possible to update it.
Also it makes little sense to update the "creator" of a work. If a "creator" (in the English literal understanding) publishes a work (a book, a picture, an NFT, anything) they remain the "creator" forever. Shakespeare cannot transfer authorship of Hamlet. As mentioned by @pqseags the transferWork()
function serves more administrative purpose and delegating admin rights. It does not necessarily need to transfer fee/royalty rights.
Thank you for these responses and insightful examples, but I agree that this design is actuall counter-intuitive and it wasn't documented anywhere. Also, I believe examples of how it works somewhere else are irrelevent. The watsons couldn't know what was the correct design for this function or it's known that when you transfer ownership the royalty is sent to the previous owner, hence, I believe it's fair to keep it valid.
Planning to reject the escalation and leave the issue as it is.
Result: Medium Has Duplicates
The protocol team fixed this issue in the following PRs/commits: https://github.com/titlesnyc/wallflower-contract-v2/pull/1
The Lead Senior Watson signed off on the fix.
xiaoming90
medium
New creators unable to update the royalty target and the fee route for their works
Summary
The new creators are unable to update the royalty target and the fee route for their works. As a result, it could lead to a loss of assets for the new creator due to the following:
Vulnerability Detail
The creator can call the
transferWork
to transfer the work to another creator. However, it was observed that after the transfer, there is still much information about the work pointing to the previous creator instead of the new creator.https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/editions/Edition.sol#L412
Following are some instances of the issues:
To aggravate the issues, creators cannot call the
Edition.setRoyaltyTarget
andFeeManager.createRoute
as these functions can only executed by EDITION_MANAGER_ROLE and [Owner, Admin], respectively.Specifically for the
Edition.setRoyaltyTarget
function that can only be executed by EDITION_MANAGER_ROLE, which is restricted and not fully trusted in the context of this audit. The new creator could have purchased the work from the previous creator, but only to find out that the malicious edition manager decided not to update the royalty target to point to the new creator's address for certain reasons, leading to the royalty fee continuing to be routed to the previous creator. In this case, it negatively impacts the new creator as it leads to a loss of royalty fee for the new creator.Impact
Loss of assets for the new creator due to the following:
Code Snippet
https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/editions/Edition.sol#L412
Tool used
Manual Review
Recommendation
Consider allowing the creators themselves to have the right to update the royalty target and the fee route for their works.