TangibleNFTV2 Manual Review Findings

TNT-01M: Incorrect Restriction of Feature Removal

Type	Severity	Location
Logical Fault		TangibleNFTV2.sol:L416

Description:

The removal of metadata and specifically features off a token ID is restricted incorrectly by a require check mandating that the feature being removed is present in the tnftMetadata contract.

This is incorrect, as a feature may be removed before it is actually removed from individual tokenId values.

Impact:

Token IDs will retain incorrect features in case a feature was pre-emptively removed from the tnftMetadata contract before being removed from individual token IDs.

Example:

Copy
407function removeMetadata(
408    uint256 tokenId,
409    uint256[] calldata _features
410) external onlyCategoryOwner(ITangibleNFT(address(this))) {
411    ITNFTMetadata tnftMetadata = ITNFTMetadata(IFactory(factory()).tnftMetadata());
412    uint256 length = _features.length;
413
414    for (uint256 i; i < length; ) {
415        uint256 feature = _features[i];
416        require(tnftMetadata.featureInType(tnftType, feature), "feature not in tfntType");
417        require(tokenFeatureAdded[tokenId][feature].added, "!exist");
418        // take last element
419        uint256 last = tokenFeatures[tokenId][tokenFeatures[tokenId].length - 1];
420        // set it to index
421        tokenFeatures[tokenId][tokenFeatureAdded[tokenId][feature].index] = last;
422        //remove from array
423        tokenFeatures[tokenId].pop();
424        // delete mapping and update index of the last
425        tokenFeatureAdded[tokenId][last].index = tokenFeatureAdded[tokenId][feature].index;
426        delete tokenFeatureAdded[tokenId][feature];
427        emit TnftFeature(tokenId, feature, false);
428
429        unchecked {
430            ++i;
431        }
432    }
433}

Recommendation:

We advise the removal of a feature to not ensure that the feature is present in the tnftType and to simply ensure that it has been added as the second require check already performs, ensuring that the feature metadata of a tokenId can be removed regardless of the tnftMetadata contract state.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The restrictive require check has been properly omitted per our recommendation, ensuring that metadata can be removed in any order.

TNT-02M: Unsafe Adjustment of Storage Decimals

Type	Severity	Location
Logical Fault		TangibleNFTV2.sol:L298-L303

Description:

The TangibleNFTV2::setStorageDecimals function will update the storageDecimals that the storagePricePerYear value should be consumed in relation to without actually updating the storagePricePerYear value.

As such, it is possible momentarily to utilize a severely over-inflated or under-inflated storagePricePerYear until it is updated to match the decimals reported by storageDecimals.

Impact:

A malicious user can exploit this misbehaviour by detecting the storageDecimals value adjustment and bribing miners to include their transaction right after the decimal adjustment, paying a significantly small storage price per year.

Example:

Copy
283/**
284 * @notice This function is used to update the storage price per year.
285 * @param _storagePricePerYear amount to pay for storage per year.
286 */
287function setStoragePricePerYear(
288    uint256 _storagePricePerYear
289) external onlyCategoryOwner(ITangibleNFT(address(this))) {
290    emit StoragePricePerYearSet(storagePricePerYear, _storagePricePerYear);
291    storagePricePerYear = _storagePricePerYear;
292}
293
294/**
295 * @notice This function is used to update `storageDecimals`.
296 * @param decimals New decimal precision for `storageDecimals`.
297 */
298function setStorageDecimals(
299    uint8 decimals
300) external onlyCategoryOwner(ITangibleNFT(address(this))) {
301    require(decimals >= 0 && decimals <= 18, "wrong");
302    storageDecimals = decimals;
303}

Recommendation:

We advise the code to either update storagePricePerYear and storageDecimals simultaneously or to convert the storagePricePerYear to the relevant storageDecimals when adjusting them, ensuring that the storagePricePerYear accuracy always matches the accuracy set via the storageDecimals value.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The storagePricePerYear value is re-calculated whenever the storageDecimals are adjusted ensuring that these variables are always in sync and thus alleviating this exhibit.