Omniscia Tangible Audit

CurrencyFeedV2 Manual Review Findings

CurrencyFeedV2 Manual Review Findings

CFV-01M: ISO 3166-1 Numeric-3 Standard Incompatibility

Description:

The CurrencyFeedV2 system and its relevant mappings are meant to be compatible with multiple ISO definitions and, in relation to country codes, the ISO 3166-1 Numeric-3 standard which denotes that country codes are meant to be represented by 3-digit codes at all times.

Impact:

The severity of this exhibit depends on the level of scrutiny that the CurrencyFeedV2 should be under in relation to its ISO 3166-1 conformity. While the system is presently incompatible with standard ISO 3166-1 systems, the effect of this deviation should solely impact off-chain services and thus be of minor impact.

Example:

contracts/helpers/CurrencyFeedV2.sol
24/// @notice This mapping is used to store a price feed oracle for a specific currency ISO numeric code.
25mapping(uint16 => AggregatorV3Interface) public currencyPriceFeedsISONum;
26
27/// @notice A premium taken by Tangible. It's tacked on top of the existing exchange rate of 2 currencies. This one is stored using the ISO numeric code for the key.
28mapping(uint16 => uint256) public conversionPremiumsISONum;
29
30/// @notice Used to store ISO curency numeric code using it's alpha code as reference.
31/// @dev i.e. ISOCurrencyCodeToNum["AUD"] = 036
32mapping(string => uint16) public ISOcurrencyCodeToNum;
33
34/// @notice Used to store ISO curency alpha code using it's numeric code as reference.
35/// @dev i.e. ISOcurrencyNumToCode[036] = "AUD"
36mapping(uint16 => string) public ISOcurrencyNumToCode;
37
38/// @notice Used to store ISO country numeric code using it's alpha code as reference.
39/// @dev i.e. ISOcountryCodeToNum["AUS"] = 036
40mapping(string => uint16) public ISOcountryCodeToNum;
41
42/// @notice Used to store ISO curency alpha code using it's numeric code as reference.
43/// @dev i.e. ISOcountryNumToCode[036] = "AUS"
44mapping(uint16 => string) public ISOcountryNumToCode;

Recommendation:

We advise the code to instead use bytes32 keys instead of the uint16 numeric type to ensure that the representation of 001 is unique and not equivalent to 01 or 1.

Alleviation (2ad448279d):

The Tangible team evaluated this exhibit and opted to not apply a remediation for it as they wish to handle the zero prefixes off-chain.

We would like to note that gas cost in terms of off-chain queries is of no concern as read-only calls do not consume any. This would permit a getter function implementation to exist that properly prefixes the entries with zero.

In any case, we consider this to be of no security concern and as such regard it as acknowledged.

Alleviation (2135afa89e):

The NatSpec of the contract has been updated to reflect that integers are utilized instead of canonical ISO 3166-1 representations. As a result of this adjustment, we consider this exhibit alleviated given that the code behaves as expected and adequately informs its readers.

CFV-02M: Inexistent Enforcement of Registration

Description:

The CurrencyFeedV2::setCurrencyFeed and CurrencyFeedV2::setCurrencyConversionPremium functions do not ensure that the ISOcurrencyCodeToNum value has been set for the specified _currency, causing a discrepancy in the data structures within the contract if it has not been so.

Impact:

A price feed or conversion premium registered in the system will not be properly attached to the ISO 3166 country alpha code.

Example:

contracts/helpers/CurrencyFeedV2.sol
81/**
82 * @notice This method is used to update the state of `currencyPriceFeeds` and `currencyPriceFeedsISONum`.
83 * @param _currency ISO-4217 alpha code. @dev I.e. if currency is Australian dollar, this value would be "AUD".
84 * @param _priceFeed Price feed contract for the specified currency.
85 */
86function setCurrencyFeed(
87 string calldata _currency,
88 AggregatorV3Interface _priceFeed
89) external onlyFactoryOwner {
90 currencyPriceFeeds[_currency] = _priceFeed;
91 // set for iso
92 currencyPriceFeedsISONum[ISOcurrencyCodeToNum[_currency]] = _priceFeed;
93}
94
95/**
96 * @notice This method is used to update the state of `conversionPremiums` and `conversionPremiumsISONum`.
97 * @param _currency ISO-4217 alpha code. @dev I.e. if currency is Australian dollar, this value would be "AUD".
98 * @param _conversionPremium A premium taken by Tangible when exchanging 2 currencies. (i.e. gbp/usd rate is 1.34, premium is 0.01)
99 */
100function setCurrencyConversionPremium(
101 string calldata _currency,
102 uint256 _conversionPremium
103) external onlyFactoryOwner {
104 conversionPremiums[_currency] = _conversionPremium;
105 // set for iso
106 conversionPremiumsISONum[ISOcurrencyCodeToNum[_currency]] = _conversionPremium;
107}

Recommendation:

We advise the functions to properly ensure that the ISOcurrencyCodeToNum have been registered, guaranteeing that the currencyPriceFeedsISONum and conversionPremiumsISONum entries are correctly set.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The CurrencyFeedV2::setCurrencyFeed and CurrencyFeedV2::setCurrencyConversionPremium functions were both updated to ensure that the ISOcurrencyCodeToNum entry has been configured for the currency in each function's input, rendering this exhibit fully alleviated.