Omniscia Tangible Audit

TNFT V2 Security Audit

Audit Report Revisions

Commit HashDateAudit Report Hash
21838badc7November 10th 20233a98d84a11
21838badc7November 10th 2023dc2cf8e22d
2ad448279dDecember 11th 20237eaa25b2b6
2ad448279dDecember 11th 20236efeb80204
1f394a00ccDecember 15th 2023fcea786500
2135afa89eDecember 22nd 2023d78a2ddd62
9515ccaa43January 21st 20241772043b02
b7fb03be2bJanuary 29th 202450053bcf49
82dc486e66March 4th 202496eacbcfef

Audit Overview

We were tasked with performing an audit of the Tangible codebase and in particular their down-sized and revised Tangible NFT codebase, supported by a variety of contracts including proprietary marketplace implementations, rent-based mechanisms, and more.

This version of the codebase has seen a substantial increase in code quality as well as a reduction in overall complexity, however, we consider certain aspects of the codebase (such as over-reliance on multi-contract calls) to remain as generic pain-points that should be addressed.

Over the course of the audit, we identified a significant price-related flaw in the marketplace that could cause users to expend more funds than they intended to for a particular NFT purchase as well as an entry-replacement flaw in the TangibleNFTDeployerV2 contract that could lead to incorrect NFTs being interacted with.

A lot of the Tangible NFT system's workflows rely on external calls as well as interactions with EIP-20 assets; we consider all these interactions to be performed with "canonical" EIP-20 assets that do not expose any re-entrancies nor impose any fees on transfers.

Some features of the system are hidden in the way the contracts interact between them and we strongly advise the Tangible team to properly document them, such as how the purchase of an unminted voucher will transfer the proceeds of a sale to the collection owner due to the way the NFT is minted to an owner's address, transferred authoritatively by the factory implementation, and triggers the EIP-721 receipt workflow of the marketplace.

We advise the Tangible team to closely evaluate all minor-and-above findings identified in the report and promptly remediate them as well as consider all optimizational exhibits identified in the report.

Post-Audit Conclusion

The Tangible team iterated through all findings within the report and provided us with a revised commit hash to evaluate all exhibits on.

We evaluated all alleviations performed by Tangible and have identified that certain exhibits have not been adequately dealt with or require a re-evaluation. We advise the Tangible team to revisit the following exhibits: PCR-01C, CFV-01M, FV2-02M, FV2-05M, SFD-03M

Additionally, the following informational findings remain unaddressed / partially addressed and should be revisited: ROV-02C, RMR-02C, GOT-05C, TNF-04C, OST-02C, OST-05C, OST-01C, TNT-01S, TNT-01C, TNT-02C, RMR-02C, GOT-02C, GOT-04C, MV2-03C, MV2-02C, MV2-01C

Post-Audit Conclusion (d8f10bb32a)

The Tangible team revisited the codebase to properly apply partially addressed optimizations as well as correct the calculation error that resulted from PCR-01C.

Most of the exhibits have been adequately addressed in the latest iteration while some, like TNT-02C, remain partially addressed.

We consider the partially addressed exhibits to be inconsequential as they purely relate to optimizations and thus can be safely acknowledged as-is.

As part of a logic update, the Tangible team performed changes to the Exchange and TNGBLV3Oracle contract, integrating with a Uniswap V3 TWAP based mechanism.

We noted that the TNGBLV3Oracle must always be compiled with a Solidity version <0.8.0 despite its pragma specification and the Tangible confirmed that this is indeed the case.

In relation to the Exchange contract, we pinpointed that a configurational percentage based offset between the TWAP's swap output and the actual swap output of the trade should be enforced rather than a fixed percentage to accommodate for potentially volatile market conditions.

Post-Audit Conclusion (1f394a00cc)

The Tangible team proceeded to implement a configurable percentage based offset to the TWAP based swap output measurement, thereby accommodating for volatile spot market conditions that would cause the swap to fail.

Based on the latest changes carried out in the codebase, we consider all outputs of the audit report properly consumed by the Tangible team and that no further findings of non-informational nature remain in the report unaddressed.

Post-Audit Conclusion (2135afa89e)

The Tangible team revisited a few of the issues that were previously acknowledged / partially alleviated; specifically: TNT-01S, CFV-01M, FV2-05M, TNT-02C, UUO-01C, UUO-02C, UUO-03C

All exhibits have been fully addressed except for UUO-03C which introduced a vulnerability as the constant introduced has an incorrect value. We advise the Tangible team to revisit this exhibit and correct its attempted alleviation.

Post-Audit Conclusion (9515ccaa43)

The Tangible team alleviated UUO-03C properly, setting the value of ZERO_UINT8 to 0 as expected.

In the same commit, the documentation, NatSpec, and overall style of the codebase was enhanced by introducing new comments, re-ordering function declarations, and applying stricter linting rules.

Given that the final open commit has been addressed, we consider all outputs of the audit engagement properly consumed by the Tangible team with no further action necessary.

Post-Audit Conclusion (b7fb03be2b)

At the behest of the Tangible team, we have updated the report to cover a new red-only function introduced in the Exchange contract, the application of an optimization we advised for TNFTMetadata::getTNFTTypesFeaturesDescriptionsAndIds, as well as whitelist-based changes in the FactoryV2 contract meant to permit a secondary address to issue buyer whitelists via the FactoryV2::whitelistBuyer function.

We did not identify any new vulnerabilities as a result of these changes and thus consider the original security guarantees upheld in the latest commit reviewed within the audit report.

Post-Audit Conclusion (82dc486e66)

The Tangible team requested a follow-up review of their latest commit hash (82dc486e66) which:

  • Removed functionality from the TangibleReaderHelperV2
  • Introduced a new TNGBLV3Oracle::getQuoteAtCurrentTick function

While we have no observations to make for the removal of functionality, the introduction of the TNGBLV3Oracle::getQuoteAtCurrentTick function led to an IERC20Metadata interface being declared within the TNGBLV3Oracle.sol file.

We advise the interface to be relocated to its own declaration, ensuring consistency in the code style across the codebase. While UUO-01S (a similar finding) was acknowledged, it was done so due to being relevant for a Chainlink dependency for which changes were not to be made.

As a final note, we would like to specify that the TNGBLV3Oracle::getQuoteAtCurrentTick function is susceptible to flash loan manipulations as it relies on the presently active tick.

We did not observe usage of the function within the codebase, however, we strongly advise the Tangible team to make sure that its usage is purely for off-chain purposes as it is insecure otherwise.

Audit Synopsis

SeverityIdentifiedAlleviatedPartially AlleviatedAcknowledged
4400
575205
141301
141400
2200

During the audit, we filtered and validated a total of 14 findings utilizing static analysis tools as well as identified a total of 77 findings during the manual review of the codebase. We strongly recommend that any minor severity or higher findings are dealt with promptly prior to the project's launch as they can introduce potential misbehaviours of the system as well as exploits.

Total Alleviations

The list below covers each segment of the audit in depth and links to the respective chapter of the report: