QA Report

[code423n4]

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/NXTPFacet.sol#L54-L59 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L46 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/CBridgeFacet.sol#L64 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/HopFacet.sol#L69

Vulnerability details

Impact

The startBridgeTokensVia... functions do not handle fee-on-transfer tokens (e.g, USDT) correctly. If some fee is deducted during the token transfer (LibAsset.transferFromERC20), the transaction will revert because of the following require check. As a result, users cannot bridge fee-on-transfer tokens via the startBridgeTokensVia... functions.

Proof of Concept

Take NXTPFacet as an example. At line 55, _nxtpData.amount is provided as the parameter of LibAsset.transferFromERC20. In cases where a fee-on-transfer token is transferred, the received amount could be less than _nxtpData.amount.

Then, at line 56, the contract ensures whether its balance is increased by exactly _nxtpData.amount, which is false in this case, causing the tx to be reverted.

NXTPFacet.sol#L54-L59

Recommended Mitigation Steps

Consider calculating the received amount of tokens after the transfer, and modify _nxtpData.amount accordingly, for example:

LibAsset.transferFromERC20(sendingAssetId, msg.sender, address(this), _nxtpData.amount);
_nxtpData.amount = LibAsset.getOwnBalance(sendingAssetId) - _sendingAssetIdBalance;

[maxklenk]

Thanks for pointing out that Fee-on-transfer tokens are not supported. We are aware of that and will document it accordingly in our documentation. We will not be able to support those tokens as long as the exchanges and bridges do not support them as well. The balance check prevents continuing the transaction with such a token.

[gzeoneth]

Downgrading to Low/QA. Treating as warden's QA Report.

[JeeberC4]

Preserving original title: Fee-on-transfer tokens are not handled correctly