revShareBps can be changed by the creator of that particular work but the old revShareBps continues to be implemented.

Summary

revShareBps can be changed by the creator of that particular work but the old revShareBps continues to be implemented. This makes changing of revShareBps redundant.

Vulnerability Detail

following is setFeeStrategy function

function setFeeStrategy(uint256 tokenId_, Strategy calldata strategy_) external {
        if (msg.sender != works[tokenId_].creator) revert Unauthorized();
        works[tokenId_].strategy = FEE_MANAGER.validateStrategy(strategy_);
    }

Now it can be seen before setting the strategy for a particular works[id] it is validated in the fee manager contract which involves various checks which are as follows

function validateStrategy(Strategy calldata strategy_)
        external
        pure
        returns (Strategy memory strategy)
    {
        // Clamp the revshare to the range of [MIN_ROYALTY_BPS...MAX_ROYALTY_BPS]
        uint16 revshareBps = strategy_.revshareBps > MAX_ROYALTY_BPS
            ? MAX_ROYALTY_BPS
            : strategy_.revshareBps < MIN_ROYALTY_BPS ? MIN_ROYALTY_BPS : strategy_.revshareBps;

        // Clamp the royalty to the range of [0...MAX_ROYALTY_BPS]
        uint16 royaltyBps =
            strategy_.royaltyBps > MAX_ROYALTY_BPS ? MAX_ROYALTY_BPS : strategy_.royaltyBps;

        strategy = Strategy({
            asset: strategy_.asset == address(0) ? ETH_ADDRESS : strategy_.asset,
            mintFee: strategy_.mintFee,
            revshareBps: revshareBps,
            royaltyBps: royaltyBps
        });
    }

So it clear from the above that even revShareBps can be changed after publishing a work. So now if revShare Bps are changed then even if the works[tokenId_].strategy is changed the change in revShareBps is not reflected anywhere which affects the collection of mint fees because revShareBps are used in the following function in order to calculate how much fees the creator and attributors receive.

function createRoute(
        IEdition edition_,
        uint256 tokenId_,
        Target[] calldata attributions_,
        address referrer_
    ) external onlyOwnerOrRoles(ADMIN_ROLE) returns (Target memory receiver) {
        Target memory creator = edition_.node(tokenId_).creator;

        if (attributions_.length == 0) {
            // No attributions, pay the creator directly
            receiver = creator;
        } else {
            // Distribute the fee among the creator and attributions
            (address[] memory targets, uint256[] memory revshares) = _buildSharesAndTargets(
        ==>        creator, attributions_, edition_.feeStrategy(tokenId_).revshareBps <==
            );

            // Create the split. The protocol retains "ownership" to enable future use cases.
            receiver = Target({
                target: splitFactory.createSplit(
                    SplitV2Lib.Split({
                        recipients: targets,
                        allocations: revshares,
                        totalAllocation: 1e6,
                        distributionIncentive: 0
                    }),
                    address(this),
                    creator.target
                    ),
                chainId: creator.chainId
            });
        }

        _feeReceivers[getRouteId(edition_, tokenId_)] = receiver;
        referrers[edition_] = referrer_;
    }

This is a valid issue because there is no clear documentation or anything where it is written that the revShare bps can't be changed.

Impact

This can cause wrong amount of fees sent to creator and attributors becuse the revShareBps has been changed but the new values are not implemented.

Code Snippet

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

Tool used

Manual Review

Recommendation

If the revShare Bps is changed then make suitable and thus change the value of the feeReceivers[getRouteId(edition, tokenId_)] accordingly.

sherlock-audit / 2024-04-titles-judging

Varun_05 - revShareBps can be changed by the creator of that particular work but the old revShareBps continues to be implemented. #350