KupiaSec - `Edition::transferWork` function doesn't change the receiver of the `_feeReceivers`

[sherlock-admin4]

KupiaSec

medium

Edition::transferWork function doesn't change the receiver of the _feeReceivers

Summary

The Edition::transferWork function is designed to transfer the work to other user and change the creator of the work. However it doesn't change the receiver of the transferred tokenId in the _feeReceivers mapping.

Vulnerability Detail

The Edition::transferWork function is designed to allow sending a token to another user. However, the current implementation of the function doesn't change the receiver of the transferred token in the _feeReceivers mapping and the new creator is not able to get the fee shares.

The current implementation of the Edition::transferWork function is as follows:

Edition.sol
412: function transferWork(address to_, uint256 tokenId_) external {
413:         Work storage work = works[tokenId_];
414:         if (msg.sender != work.creator) revert Unauthorized();
415: 
416:         // Transfer the work to the new creator
417:         work.creator = to_;
418: 
419:         emit WorkTransferred(address(this), tokenId_, to_);
420:     }

As evident from the provided code, at L417 the creator is changed.

When publishing a new work, the creator is set as the first target in the splitFactory.

TitlesCore.sol
120: function _publish(Edition edition_, WorkPayload memory work_, address referrer_)
121:         internal
122:         returns (uint256 tokenId)
123:     {
124:         // Publish the new Work in the Edition
125:         // wake-disable-next-line reentrancy
126:         tokenId = edition_.publish(
127:             work_.creator.target,
128:             work_.maxSupply,
129:             work_.opensAt,
130:             work_.closesAt,
131:             work_.attributions,
132:             work_.strategy,
133:             work_.metadata
134:         );
135: 
136:         // Collect the creation fee
137:         // wake-disable-next-line reentrancy
138:         feeManager.collectCreationFee{value: msg.value}(edition_, tokenId, msg.sender);
139: 
140:         // Create the fee route for the new Work
141:         // wake-disable-next-line reentrancy
142:         Target memory feeReceiver = feeManager.createRoute( //@audit-info create route
143:             edition_, tokenId, _attributionTargets(work_.attributions), referrer_
144:         );
145: 
146:         // Set the royalty target for the new Work
147:         // wake-disable-next-line reentrancy
148:         edition_.setRoyaltyTarget(tokenId, feeReceiver.target);
149:     }

The implementation of the FeeManager::createRoute is as follows:

FeeManager.sol
125: function createRoute(
126:         IEdition edition_,
127:         uint256 tokenId_,
128:         Target[] calldata attributions_,
129:         address referrer_
130:     ) external onlyOwnerOrRoles(ADMIN_ROLE) returns (Target memory receiver) {
131:         Target memory creator = edition_.node(tokenId_).creator;
132: 
133:         if (attributions_.length == 0) {
134:             // No attributions, pay the creator directly
135:             receiver = creator;
136:         } else {
137:             // Distribute the fee among the creator and attributions
138:             (address[] memory targets, uint256[] memory revshares) = _buildSharesAndTargets(//@audit-info creator = targets[0]
139:                 creator, attributions_, edition_.feeStrategy(tokenId_).revshareBps
140:             );
141: 
142:             // Create the split. The protocol retains "ownership" to enable future use cases.
143:             receiver = Target({ //@audit-info creator is kept in the receiver
144:                 target: splitFactory.createSplit(
145:                     SplitV2Lib.Split({
146:                         recipients: targets,
147:                         allocations: revshares,
148:                         totalAllocation: 1e6,
149:                         distributionIncentive: 0
150:                     }),
151:                     address(this),
152:                     creator.target
153:                     ),
154:                 chainId: creator.chainId
155:             });
156:         }
157: 
158:         _feeReceivers[getRouteId(edition_, tokenId_)] = receiver; //@audit set receiver here
159:         referrers[edition_] = referrer_;
160:     }

At L138 the creator is set as the targets[0].

FeeManager.sol
476: function _buildSharesAndTargets(
477:         Target memory creator,
478:         Target[] memory attributions,
479:         uint32 revshareBps
480:     ) internal pure returns (address[] memory targets, uint256[] memory shares) {
481:         uint32 attributionShares = uint32(attributions.length);
482:         uint32 attributionRevShare = revshareBps * 100 / attributionShares;
483:         uint32 creatorShare = 1e6 - (attributionRevShare * attributionShares);
484: 
485:         // Build the targets and shares arrays using this layout:
486:         // - targets: [creator, ...attributions]
487:         // - shares: [creatorShare, ...attributionShares]
488:         targets = new address[](attributionShares + 1);
489:         shares = new uint256[](attributionShares + 1);
490: 
491:         targets[0] = creator.target; //@audit creator is set as targets[0]
492:         shares[0] = creatorShare;
493: 
494:         for (uint8 i = 0; i < attributionShares; i++) {
495:             targets[i + 1] = attributions[i].target;
496:             shares[i + 1] = attributionRevShare;
497:         }
498:     }

As evident from provided code, the creator is set as targets[0] and saved in the _feeReceiver mapping. However the transferWork doesn't change the creator of the mapping and new creator is not able receive the fee share.

On the other hand, in [Edition::publish]() function, the edges are created according to the creator from L127 below.

File: Edition.sol
103:     function publish(
104:         address creator_,
105:         uint256 maxSupply_,
106:         uint64 opensAt_,
107:         uint64 closesAt_,
108:         Node[] calldata attributions_,
109:         Strategy calldata strategy_,
110:         Metadata calldata metadata_
111:     ) external override onlyRoles(EDITION_MANAGER_ROLE) returns (uint256 tokenId) {
             [...]
125:         for (uint256 i = 0; i < attributions_.length; i++) {
126:             // wake-disable-next-line reentrancy, unchecked-return-value
127:             GRAPH.createEdge(_node, attributions_[i], attributions_[i].data);
128:         }
129: 
130:         emit Published(address(this), tokenId);
131:     }

So, if the creator is changed by calling transferWork function, the edges related to the creator must be recreated.

Impact

The new creator is not able to receive the fee share and edges are not updated according to new creator.

Code Snippet

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

Tool used

Manual Review

Recommendation

It is recommended to update the receiver of the _feeReceivers and edges according to new creator.

Duplicate of #283

[ccashwell]

Won't fix, this is expected behavior