QA Report

[code423n4]

1. typos

instead of : _potentialReplcia
it should be : _potentialReplica
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/BaseConnextFacet.sol#L137
instead of bridgable
it should be: bridgeable
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/BridgeFacet.sol#L309
instead of : addressess
use : addresses
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/BridgeFacet.sol#L968
instead of : sournce
use: source
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol#L163
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol#L176
instead of: rlayer
use: relayer
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/RelayerFacet.sol#L35
instead of specicifed
use: specified
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/RoutersFacet.sol#L575
instead of : funcition
use:function
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/upgrade-initializers/DiamondInit.sol#L31
instead of : callabale
use: callable
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/helpers/Executor.sol#L17

2. import not needed

Ierc20.sol can be imported from saferc20.sol
take out the Ierc20.sol and import the Iec20 from safeerc20.sol
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/BridgeFacet.sol#L5
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/helpers/SponsorVault.sol#L5

best practice to remove todo comments from code it can tell an attacker inside info that wasnt meant to be send

https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/BridgeFacet.sol#L608
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/BridgeFacet.sol#L707
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/helpers/Executor.sol#L7
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/libraries/LibConnextStorage.sol#L306

after calling call function you shouldn't put in a if statment because if the call fails then it just skips execution of that if statment ,logic can brake and loss of funds

instead put int a require statment like it should be to make sure the call function happens.

remove code in comments its best practice

https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/DiamondLoupeFacet.sol#L18-L23
https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/StableSwap.sol#L112-L113

no check on address zero it can loose of funds

https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/DiamondLoupeFacet.sol#L32
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/AssetFacet.sol#L112
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/AssetFacet.sol#L100
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/RelayerFacet.sol#L88
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/RelayerFacet.sol#L101
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/RelayerFacet.sol#L112
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/promise/PromiseRouter.sol#L155

make the erc20 not upgradeable

A change to the token semantics can break any smart contract that depends on the past behaviour.
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/PortalFacet.sol#L4

dont use charot of solidity version you can use one for testing and one for deployment and there can be a bug in deployment version.

https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/upgrade-initializers/DiamondInit.sol#L2

Most contracts use an init pattern (instead of a constructor) to initialize contract parameters. Unless these are enforced to be atomic with contact deployment via deployment script or factory contracts, they are susceptible to front-running race conditions where an attacker/griefer can front-run (cannot access control because admin roles are not initialized) to initially with their own (malicious) parameters upon detecting (if an event is emitted) which the contract deployer has to redeploy wasting gas and risking other transactions from interacting with the attacker-initialized contract.

Many init functions (those in auction contracts) do not have an explicit event emission which makes monitoring such scenarios harder. All of them have re-init checks; while many are explicit some (those in auction contracts) have implicit reinit checks in initAccessControls() which is better if converted to an explicit check in the main init function itself.

Proof of Concept

https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/facets/upgrade-initializers/DiamondInit.sol#L43
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/connext/helpers/LPToken.sol#L21
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/promise/PromiseRouter.sol#L146
https://github.com/code-423n4/2022-06-connext/blob/20f86d58444d7c8178735ada7e456a3112116e54/contracts/contracts/core/relayer-fee/RelayerFeeRouter.sol#L80

Recommended Mitigation Steps

Ensure atomic calls to init functions along with deployment via robust deployment scripts or factory contracts. Emit explicit events for initializations of contracts. Enfore prevention of re-initializations via explicit setting/checking of boolean initialized variables in the main init function instead of downstream function checks.

token can be more than 18 deciamls like 24 deciamls and that function will fail and it can more attack vectors for an attacker.

certain variable can fail and not get the price from dex
https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L106
https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L103
https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L134

[jakekidd]

some of these are missing examples

some require more details

the >18 decimals is a level 2: Med Risk issue, but a duplicate of a number of other ">18 decimals" level 2 issues.