Omniscia Tangible Audit
TNFT V2 Security Audit
Audit Report Revisions
Commit Hash | Date | Audit Report Hash |
---|---|---|
21838badc7 | November 10th 2023 | 3a98d84a11 |
21838badc7 | November 10th 2023 | dc2cf8e22d |
2ad448279d | December 11th 2023 | 7eaa25b2b6 |
2ad448279d | December 11th 2023 | 6efeb80204 |
1f394a00cc | December 15th 2023 | fcea786500 |
2135afa89e | December 22nd 2023 | d78a2ddd62 |
9515ccaa43 | January 21st 2024 | 1772043b02 |
b7fb03be2b | January 29th 2024 | 50053bcf49 |
82dc486e66 | March 4th 2024 | 96eacbcfef |
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
Severity | Identified | Alleviated | Partially Alleviated | Acknowledged |
---|---|---|---|---|
4 | 4 | 0 | 0 | |
57 | 52 | 0 | 5 | |
14 | 13 | 0 | 1 | |
14 | 14 | 0 | 0 | |
2 | 2 | 0 | 0 |
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: