sherlock-audit / 2024-04-titles-judging

2 stars 2 forks source link

i3arba - `TitlesCore.sol` implements logic wrongly by givin the ownership of the edition to the `TitlesCore.sol` contract #344

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

i3arba

high

TitlesCore.sol implements logic wrongly by givin the ownership of the edition to the TitlesCore.sol contract

Summary

As the document states, the EDITION_MANAGER_ROLE has the ownership of the edition and can adjust critical information of it. While EDITION_PUBLISHER_ROLE can perform limited action.

" EDITION_MANAGER_ROLE (Restricted) => On an Edition, this role can:

  1. Publish a new work with any desired configuration (publish).
    • This is the only way to create new works after the Edition is created.
      1. Mint promotional copies of any work (promoMint).
    • There are no limitations on this action aside from the work's supply cap and minting period.
  2. Set the Edition's ERC2981 royalty receiver (setRoyaltyTarget).
    • This is the only way to change the royalty receiver for the Edition.
  3. Grant or revoke any role to/from any address (grantRole, revokeRole).

EDITION_PUBLISHER_ROLE (Restricted) => On TitlesCore, this role can:

  1. Publish a new work under any Edition for which they have been granted the role (i.e. edition.hasAnyRole(msg.sender, EDITION_PUBLISHER_ROLE) is true) (publish).
    • After auth, the request is passed to the Edition contract for further handling. "

Vulnerability Detail

Edition.sol::initialize" function undermines the Edition's owner power. All the administrative power is passed toTitlesCore.solwhen the function grants theEDITION_MANAGER_ROLEtoTitlesCore.sol`.

PoC ```solidity function test_possibleDoS() public { graph = new TitlesGraph(address(this), address(this)); Edition edition = titlesCore.createEdition{value: 100000000000000}( LibZip.cdCompress( abi.encode(TitlesCore.EditionPayload({ work: TitlesCore.WorkPayload({ creator: Target({chainId: block.chainid, target: address(this)}), attributions: new Node[](0), maxSupply: 100, opensAt: uint64(block.timestamp), closesAt: uint64(block.timestamp + 3 days), strategy: Strategy({ asset: ETH_ADDRESS, mintFee: 0.01 ether, revshareBps: 1000, royaltyBps: 250 }), metadata: Metadata({label: "Werk", uri: "https://ipfs.io/{{hash}}", data: new bytes(0)}) }), metadata: Metadata({label: "Test Edition", uri: "https://ipfs.io/{{hash}}", data: new bytes(0)})})) ), address(titlesCore) ); uint256 role = edition.rolesOf(address(titlesCore)); uint256 rightOwner = edition.rolesOf(address(this)); assertEq(role, 2048); //ADMIN_ROLE assertEq(rightOwner, 8192); //EDITION_MINTER_ROLE } ```

Impact

Edition owners lose ownership of their collection when it's created, being limited to only publishing new work without performing any change that otherwise it would be allowed to.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/editions/Edition.sol#L89-L90

Tool used

Manual Review

Recommendation

Code Adjustment ```diff function initialize( FeeManager feeManager_, TitlesGraph graph_, address owner_, address controller_, Metadata calldata metadata_ ) external initializer { _initializeOwner(owner_); FEE_MANAGER = feeManager_; GRAPH = graph_; - _grantRoles(controller_, EDITION_MANAGER_ROLE); + _grantRoles(owner_, EDITION_MANAGER_ROLE); - _grantRoles(owner_, EDITION_PUBLISHER_ROLE); + _grantRoles(controller_, EDITION_PUBLISHER_ROLE); _metadata[0] = metadata_; } ```