alexzoid - Inconsistent Edge ID Generation after Work Transfer

[sherlock-admin4]

alexzoid

medium

Inconsistent Edge ID Generation after Work Transfer

Summary

Transferring a work in the Edition contract leads to discrepancies in edge ID generation, causing issues with subsequent operations on edges like acknowledgment or unacknowledgment.

Vulnerability Detail

The transferWork function in the Edition contract updates the creator of a work, which alters the return value of node(uint256 tokenId). Since edge IDs are generated based on node information, changes to the node affect the consistency of edge IDs. This discrepancy means that after a work is transferred, the previously generated edge IDs no longer correspond accurately to their associated nodes, preventing the correct execution of functions that rely on these IDs.

Impact

This is a medium severity issue as it disrupts the functionality of acknowledging or unacknowledging edges after a work transfer.

Code Snippet

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

function transferWork(address to_, uint256 tokenId_) external {
    Work storage work = works[tokenId_];
    if (msg.sender != work.creator) revert Unauthorized();

    // Transfer the work to the new creator
    work.creator = to_;

    emit WorkTransferred(address(this), tokenId_, to_);
}

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

    function node(uint256 tokenId) public view returns (Node memory) {
        return Node({
            nodeType: NodeType.TOKEN_ERC1155,
            creator: Target({target: works[tokenId].creator, chainId: block.chainid}),
            entity: Target({target: address(this), chainId: block.chainid}),
            data: abi.encode(tokenId)
        });
    }

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/interfaces/IEdgeManager.sol#L22-L36

    /// @notice Acknowledges an {Edge} with the given data.
    /// @param edgeId_ The ID of the {Edge} to acknowledge.
    /// @param data_ The data associated with the acknowledgment.
    /// @return edge The acknowledged {Edge}.
    function acknowledgeEdge(bytes32 edgeId_, bytes calldata data_)
        external
        returns (Edge memory edge);

    /// @notice Unacknowledges an {Edge} with the given data.
    /// @param edgeId_ The ID of the {Edge} to unacknowledge.
    /// @param data_ The data associated with the unacknowledgment.
    /// @return edge The unacknowledged {Edge}.
    function unacknowledgeEdge(bytes32 edgeId_, bytes calldata data_)
        external
        returns (Edge memory edge);
}

Tool used

Manual Review

Recommendation

Consider updating the edge ID generation logic to be less dependent on mutable attributes or explicitly handle edge ID updates when works are transferred.

Duplicate of #283

[alexzoid-eth]

Escalate

This issue is a valid excluded dup of #283

[sherlock-admin3]

Escalate

This issue is a valid excluded dup of #283

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.

[WangSecurity]

Unfortunately, I believe this report doesn't identify any impact in the smart contracts and doesn't explain what will be the effect of the incorrect execution and what it leads to. Hence, I believe it's insufficient to be a duplicate of #283

[Evert0x]

Result: Medium Duplicate of #283

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• alexzoid-eth: accepted