TangibleNFTDeployerV2 Manual Review Findings

TND-01M: Inexistent Prevention of Symbol Overwrite

Type	Severity	Location
Logical Fault		TangibleNFTDeployerV2.sol:L47-L74

Description:

The TangibleNFTDeployerV2::deployTnft function permits a configuration with different parameters but the same symbol as a previous deployment to be deployed, causing the tnftBeaconProxy data entry to be replaced potentially maliciously.

Impact:

Depending on the usage of the tnftBeaconProxy by off-chain services, it may be possible to trick users into interacting with Tangible NFT collections owned by an entirely different party.

Example:

Copy
47function deployTnft(
48    string calldata name,
49    string calldata symbol,
50    string calldata uri,
51    bool isStoragePriceFixedAmount,
52    bool storageRequired,
53    bool _symbolInUri,
54    uint256 _tnftType
55) external onlyFactory returns (ITangibleNFT) {
56    BeaconProxy newTangibleNFTV2Beacon = new BeaconProxy(
57        address(beacon),
58        abi.encodeWithSelector(
59            TangibleNFTV2.initialize.selector,
60            factory(),
61            name,
62            symbol,
63            uri,
64            isStoragePriceFixedAmount,
65            storageRequired,
66            _symbolInUri,
67            _tnftType
68        )
69    );
70    // store beacon proxy address in mapping
71    tnftBeaconProxy[symbol] = address(newTangibleNFTV2Beacon);
72
73    return ITangibleNFT(address(newTangibleNFTV2Beacon));
74}

Recommendation:

We advise the code to ensure that the tnftBeaconProxy data entry for the requested symbol is empty, disallowing a deployment of a Tangible NFT with the same symbol as a previous one.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The require check recommended has been properly introduced to the function ensuring that the tnftBeaconProxy of a symbol already configured cannot be overwritten.