Omniscia Tangible Audit

SellFeeDistributorV2 Manual Review Findings

SellFeeDistributorV2 Manual Review Findings

SFD-01M: Inexistent Protection of Fee Distribution

Description:

The SellFeeDistributorV2::distributeFee function does not impose any access control.

Impact:

While it does not lead to any active vulnerability manifesting, it is still advisable to reduce the potential attack surface of the contract by restricting callers of its balance-managing SellFeeDistributorV2::distributeFee function.

Example:

contracts/SellFeeDistributorV2.sol
123function distributeFee(IERC20 _paymentToken, uint256 _feeAmount) external {
124    _distributeFee(_paymentToken, _feeAmount);
125}

Recommendation:

We advise it to be solely invoke-able by the MarketplaceV2 implementation, based on the workflow employed in MarketplaceV2::_buy.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The Tangible team specifies that this is intentional to allow external integration of the fee distribution workflow.

As a result, we consider this exhibit to pertain to desirable behaviour and thus nullified.

SFD-02M: Inexistent Tangible Token Burn Workflow (Satellite Chain)

Description:

The SellFeeDistributorV2::_distributeFee function will only burn the TNGBL tokens it acquires if the contract has been deployed on the mainnet whilst in any other case the funds will simply remain within the contract.

Impact:

The contract will significantly misbehave if it ever receives a fee distribution whereby the _paymentToken is the TNGBL asset as it will re-distribute supposedly "burned" TNGBL units.

Example:

contracts/SellFeeDistributorV2.sol
134function _distributeFee(IERC20 _paymentToken, uint256 _feeAmount) internal {
135    //take 66.6666% and send to revenueShare
136    uint256 amountForRevenue = (_feeAmount * revenuePercent) / FULL_PORTION;
137    uint256 amountForBurn = _feeAmount - amountForRevenue;
138    if (address(_paymentToken) != address(USDC)) {
139        //we need to convert the payment token to usdc
140        _paymentToken.approve(address(exchange), amountForRevenue);
141        amountForRevenue = exchange.exchange(
142            address(_paymentToken),
143            address(USDC),
144            amountForRevenue,
145            exchange.quoteOut(address(_paymentToken), address(USDC), amountForRevenue)
146        );
147    }
148    USDC.safeTransfer(revenueShare, amountForRevenue);
149    emit FeeDistributed(revenueShare, amountForRevenue);
150

151    //convert 33.334% to tngbl and burn it
152    // exchange usdc for tngbl
153    _paymentToken.approve(address(exchange), amountForBurn);
154    uint256 tngblToBurn = exchange.exchange(
155        address(_paymentToken),
156        address(TNGBL),
157        amountForBurn,
158        exchange.quoteOut(address(_paymentToken), address(TNGBL), amountForBurn)
159    );
160

161    if (isMainnet) {
162        //burn the tngbl
163        TNGBL.approve(address(this), tngblToBurn);
164        ERC20Burnable(address(TNGBL)).burn(tngblToBurn);
165

166        emit TangibleBurned(tngblToBurn);
167    }
168}

Recommendation:

We advise the code to instead transfer the funds to a known "dead" address, such as 0xdeaDDeADDEaDdeaDdEAddEADDEAdDeadDEADDEaD, properly taking the TNGBL units acquired out of circulation.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The code has been updated to properly distribute the tngblToBurn funds to the 0xdeaDDeADDEaDdeaDdEAddEADDEAdDeadDEADDEaD address in case the contract is deployed in a satellite chain, alleviating this exhibit in full.

SFD-03M: Insecure On-Chain Trades

Description:

The SellFeeDistributorV2::_distributeFee function will insecurely perform two swaps to the USDC and TNGBL asset from the _paymentToken as it will specify the minimum-outputs it accepts using on-chain data, as evidenced by the Exchange::quoteOut implementation.

Impact:

Presently, all funds swapped via the SellFeeDistributorV2::_distributeFee function are susceptible to on-chain slippage attacks, such as sandwich attacks.

Example:

contracts/SellFeeDistributorV2.sol
134function _distributeFee(IERC20 _paymentToken, uint256 _feeAmount) internal {
135    //take 66.6666% and send to revenueShare
136    uint256 amountForRevenue = (_feeAmount * revenuePercent) / FULL_PORTION;
137    uint256 amountForBurn = _feeAmount - amountForRevenue;
138    if (address(_paymentToken) != address(USDC)) {
139        //we need to convert the payment token to usdc
140        _paymentToken.approve(address(exchange), amountForRevenue);
141        amountForRevenue = exchange.exchange(
142            address(_paymentToken),
143            address(USDC),
144            amountForRevenue,
145            exchange.quoteOut(address(_paymentToken), address(USDC), amountForRevenue)
146        );
147    }
148    USDC.safeTransfer(revenueShare, amountForRevenue);
149    emit FeeDistributed(revenueShare, amountForRevenue);
150

151    //convert 33.334% to tngbl and burn it
152    // exchange usdc for tngbl
153    _paymentToken.approve(address(exchange), amountForBurn);
154    uint256 tngblToBurn = exchange.exchange(
155        address(_paymentToken),
156        address(TNGBL),
157        amountForBurn,
158        exchange.quoteOut(address(_paymentToken), address(TNGBL), amountForBurn)
159    );
160

161    if (isMainnet) {
162        //burn the tngbl
163        TNGBL.approve(address(this), tngblToBurn);
164        ERC20Burnable(address(TNGBL)).burn(tngblToBurn);
165

166        emit TangibleBurned(tngblToBurn);
167    }
168}

Recommendation:

We advise the code to instead utilize a decentralized way of measuring the outputs of the swaps, potentially via the usage of Chainlink oracles or TWAP mechanisms, ensuring that the trades performed by the SellFeeDistributorV2 are not susceptible to sandwich attacks.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The code of the Exchange::quoteOut function was significantly refactored to utilize a TWAP mechanism, ensuring that the output estimations it yields are less susceptible to manipulation.

While we consider this solution adequate for small exchanges, we advise the Tangible team to incorporate a manipulation-less mechanism to estimate outputs if they deem the exchanges they are used in to be of high value (i.e. surpassing 6 figures).

We will mark this exhibit as alleviated and will request the Tangible to revisit in case they wish to perform further changes.

Prev

RentManager.sol (RMR-M)

Next

TNFTMetadata.sol (TNF-M)

On this page