sherlock-audit / 2024-04-titles-judging

1 stars 1 forks source link

Bauchibred - The restricted Edition owner is allowed to have access to context they shouldn't be allowed to #398

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

Bauchibred

high

The restricted Edition owner is allowed to have access to context they shouldn't be allowed to

Summary

See below

Vulnerability Detail

First note that an edition owner/ and work creators are only trusted in the context of their works, this can be deduced subtly from the readme and clearly reiterated/confirmed from the discussion with the sponsors in the discord chat below, i.e

Q: Hey @pqseags and @ccashh , in the contest readme (https://audits.sherlock.xyz/contests/326), it was unclear if the following two (2) roles are considered Trusted or Restricted. Do you mind letting us know whether they are Trusted or Restricted? Thanks!

  1. Editions have an Ownable owner - Edition's owner
  2. Works within an Edition have a creator - Edition's creator

Would be key to note that this was the reply from the protocol team after asking the question:

pqseags: Edition owners and Work creators are trusted in the context of their own respective Editions/Works but not in the context of the broader ecosystem including other Editions/Works

Now take a look at https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/editions/Edition.sol#L377-L387

    function setMetadata(uint256 id_, Metadata calldata metadata_) external {
        //@audit-ok metadata issue? owner can set a malicious metadata for users
        // Only the owner can update the Edition metadata
        if (id_ == 0 && msg.sender != owner()) revert Unauthorized();

        // Only the creator can update the work metadata
        if (id_ > 0 && msg.sender != works[id_].creator) revert Unauthorized();

        _metadata[id_] = metadata_;
    }

We can see that in this case the Edition owner who is not trusted in the context of the broader ecosystem actually has the access of setting the metadata for the id == 0 id, since they aren't trusted to do this, this then allows them after turning malicious to be able to set the data in the metadata to malicious ones, from having malicious strings/links as the uri to any sort of malicious redirections that could be done via

Impact

The admin who's not trusted in the current context for setting the metadata for the 0th id can actually point this metadata to any malicious data when they see fit, break core ideologies in protocol, and even users trust on the platform.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/editions/Edition.sol#L377-L387

Tool used

Manual Review

Recommendation

Ensure that the protocol team and its users are aware of the risks of such an event and develop a contingency plan to manage it!