QA Report

[code423n4]

Summary

I've found 7 low-risk issues and some non-critical ones.

Low-Risk Issues

1. Wrong comments
   2022-06-connext\contracts\contracts\core\connext\helpers\StableSwap.sol#L223
   "@notice Calculate the amount of tokens the user must transfer"

2022-06-connext\contracts\contracts\core\connext\helpers\StableSwap.sol#L227
"@return amount of tokens the user must transfer"

2022-06-connext\contracts\contracts\core\connext\helpers\StableSwap.sol#L240
"@notice Calculate the amount of tokens the user must transfer"

2022-06-connext\contracts\contracts\core\connext\libraries\LibConnextStorage.sol#L299
Currently, it's the same as "portalDebt" and it must be changed for "portalFeeDebt".

2022-06-connext\contracts\contracts\core\connext\libraries\SwapUtils.sol#L209
"// xpReduced[i] = xp[i] - dxExpected * fee / FEE_DENOMINATOR"

2022-06-connext\contracts\contracts\core\connext\libraries\SwapUtils.sol#L790
"Swap needs more than max tokens"

2. safeApprove() is deprecated, use safeIncreaseAllowance() and safeDecreaseAllowance() instead
   2022-06-connext\contracts\contracts\core\connext\libraries\AssetLogic.sol#L347

3. Missing events for onlyAdmin functions that change important parameters
   2022-06-connext\contracts\contracts\core\connext\facets\PortalFacet.sol#L68

4. Check address(0) before transfer funds even if they are onlyAdmin functions.
   2022-06-connext\contracts\contracts\core\connext\facets\RelayerFacet.sol#L172
   2022-06-connext\contracts\contracts\core\connext\helpers\SponsorVault.sol#L260
   2022-06-connext\contracts\contracts\core\connext\helpers\SponsorVault.sol#L293
   2022-06-connext\contracts\contracts\core\connext\helpers\SponsorVault.sol#L296
   2022-06-connext\contracts\contracts\core\connext\libraries\AssetLogic.sol#L142
   2022-06-connext\contracts\contracts\core\connext\libraries\AssetLogic.sol#L145
   2022-06-connext\contracts\contracts\core\connext\libraries\SwapUtils.sol#L1059

5. PortalFacet.getAavePortalFeeDebt() returns wrong results.
   2022-06-connext\contracts\contracts\core\connext\facets\PortalFacet.sol#L39
   It must return "s.portalFeeDebt[_transferId]".

6. Check address(0) when change admin role.
   2022-06-connext\contracts\contracts\core\connext\helpers\ConnextPriceOracle.sol#L168

7. Inconsistent implementation of "adminFee" and "swapFee".
   2022-06-connext\contracts\contracts\core\connext\libraries\SwapUtils.sol#L1071
   2022-06-connext\contracts\contracts\core\connext\libraries\SwapUtils.sol#L1084
   2022-06-connext\contracts\contracts\core\connext\facets\StableSwapFacet.sol#L432-L433
   2022-06-connext\contracts\contracts\core\connext\helpers\StableSwap.sol#L97-98

It checks adminFee < MAX_ADMIN_FEE, swapFee < MAX_SWAP_FEE in 2 places but checks
adminFee <= MAX_ADMIN_FEE, swapFee <= MAX_SWAP_FEE in other place.

Non-critical Issues

1. Natspec incomplete
   2022-06-connext\contracts\contracts\core\connext\facets\AssetFacet.sol#L132
   @param _stableSwapPool

2022-06-connext\contracts\contracts\core\connext\facets\PortalFacet.sol#L80
@param _transferId

2022-06-connext\contracts\contracts\core\connext\facets\StableSwapFacet.sol#L255
@param minAmountOut
@param deadline
@return

2022-06-connext\contracts\contracts\core\connext\facets\StableSwapFacet.sol#L279
@param maxAmountIn
@param deadline
@return

2022-06-connext\contracts\contracts\core\connext\helpers\StableSwap.sol#L319
@return

2022-06-connext\contracts\contracts\core\connext\helpers\StableSwap.sol#L336
@return

2022-06-connext\contracts\contracts\core\connext\helpers\StableSwap.sol#L354
@return

2022-06-connext\contracts\contracts\core\connext\libraries\AssetLogic.sol#L226
@param _maxIn
@return

2022-06-connext\contracts\contracts\core\connext\libraries\AssetLogic.sol#L262
@param _slippageTol
@return

2022-06-connext\contracts\contracts\core\connext\libraries\AssetLogic.sol#L304
@param _maxIn
@return

2022-06-connext\contracts\contracts\core\connext\libraries\LibConnextStorage.sol#L77
@param routerSignatures

2022-06-connext\contracts\contracts\core\connext\libraries\LibConnextStorage.sol#L100
@param approvedForPortalRouters

2022-06-connext\contracts\contracts\core\connext\libraries\SwapUtils.sol#L173
@param totalSupply

2022-06-connext\contracts\contracts\core\connext\libraries\SwapUtils.sol#L484
@param balances

2022-06-connext\contracts\contracts\core\connext\libraries\SwapUtils.sol#L515
@param balances

2. Event should use indexed fields if there are three or more fields
   2022-06-connext\contracts\contracts\core\connext\facets\AssetFacet.sol#L27
   2022-06-connext\contracts\contracts\core\connext\facets\AssetFacet.sol#L35
   2022-06-connext\contracts\contracts\core\connext\facets\AssetFacet.sol#L44
   2022-06-connext\contracts\contracts\core\connext\facets\AssetFacet.sol#L55
   2022-06-connext\contracts\contracts\core\connext\facets\BridgeFacet.sol#L179
   2022-06-connext\contracts\contracts\core\connext\facets\BridgeFacet.sol#L187
   2022-06-connext\contracts\contracts\core\connext\facets\BridgeFacet.sol#L195
   2022-06-connext\contracts\contracts\core\connext\facets\RelayerFacet.sol#L25
   2022-06-connext\contracts\contracts\core\connext\helpers\ConnextPriceOracle.sol#L66-L67
   2022-06-connext\contracts\contracts\core\connext\helpers\SponsorVault.sol#L73
   2022-06-connext\contracts\contracts\core\connext\helpers\SponsorVault.sol#L78
   2022-06-connext\contracts\contracts\core\connext\helpers\SponsorVault.sol#L83
   2022-06-connext\contracts\contracts\core\connext\helpers\SponsorVault.sol#L88
   2022-06-connext\contracts\contracts\core\connext\helpers\SponsorVault.sol#L93
   2022-06-connext\contracts\contracts\core\connext\helpers\SponsorVault.sol#L98
   2022-06-connext\contracts\contracts\core\connext\helpers\SponsorVault.sol#L108
   2022-06-connext\contracts\contracts\core\connext\helpers\SponsorVault.sol#L113
   2022-06-connext\contracts\contracts\core\connext\libraries\AmplificationUtils.sol#L17
   2022-06-connext\contracts\contracts\core\connext\libraries\LibDiamond.sol#L69
   2022-06-connext\contracts\contracts\core\connext\libraries\LibDiamond.sol#L81
   2022-06-connext\contracts\contracts\core\connext\libraries\LibDiamond.sol#L92
   2022-06-connext\contracts\contracts\core\promise\PromiseRouter.sol#L78
   2022-06-connext\contracts\contracts\core\relayer-fee\RelayerFeeRouter.sol#L50

3. Typo
   2022-06-connext\contracts\contracts\core\connext\helpers\StableSwap.sol#L244
   @return amount of tokens the user "must(has to)" transfer

2022-06-connext\contracts\contracts\core\connext\libraries\AssetLogic.sol#L253
@notice Swaps assetIn "t" assetOut using the stored stable swap or internal swap pool

2022-06-connext\contracts\contracts\core\connext\libraries\AssetLogic.sol#L295
@notice Swaps assetIn "t" assetOut using the stored stable swap or internal swap pool

2022-06-connext\contracts\contracts\core\connext\libraries\AssetLogic.sol#L331
NOTE: this is less efficient "then" relying on the swapInternalOut revert, but makes it easier

2022-06-connext\contracts\contracts\core\connext\libraries\SwapUtils.sol#L917
minAmounts must match "poolTokens" => "pooledTokens"?

4. Needless import
   2022-06-connext\contracts\contracts\core\connext\helpers\ConnextPriceOracle.sol#L4
   2022-06-connext\contracts\contracts\core\connext\libraries\AmplificationUtils.sol#L5
   2022-06-connext\contracts\contracts\core\connext\libraries\SwapUtils.sol#L4

5. Immutable addresses should be 0-checked
   2022-06-connext\contracts\contracts\core\connext\helpers\Executor.sol#L48

6. Open TODOs
   2022-06-connext\contracts\contracts\core\connext\facets\BridgeFacet.sol#L492
   2022-06-connext\contracts\contracts\core\connext\facets\BridgeFacet.sol#L579
   2022-06-connext\contracts\contracts\core\connext\facets\BridgeFacet.sol#L1027
   2022-06-connext\contracts\contracts\core\connext\helpers\Executor.sol#L7
   2022-06-connext\contracts\contracts\core\connext\libraries\LibConnextStorage.sol#L303

[jakekidd]

Low-Risk issues don't seem to carry any risk or are invalid

Non-critical (QA) are great