QA Report

[code423n4]

#1 Unused variable
Impact
efficiency code and reduce gas cost

Proof of Concept
https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/facets/BridgeFacet.sol#L859

Tool Used
Remix

Recommended Mitigation Steps
remove unnecessary code. i suggest to change

  (bool success, bytes memory returnData) = s.executor.execute

to

  (bool success, ) = s.executor.execute

#2 Missing param define _transferId
impact
repayAavePortal function have natspec comment which is missing the newOwner function parameter. Issues with comments are low risk based on Code4rena risk categories.

Proof of Concept
https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/facets/PortalFacet.sol#L74-L79

Tool
Manual Review

Recommended Mitigation Steps
Add natspec comments include transferId parameter in repayAavePortal function.

#3 Router must be indexed

Impact
event is missing indexed fields.

Proof of Concept
https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/facets/RoutersFacet.sol#L117

https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/facets/RoutersFacet.sol#L124

Tool
Manual review

Recommendation Mitigation Steps
Add indexed at router.

#4 canonicalId must be indexed

Impact
event is missing indexed fields.

Proof of Concept
https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/facets/RoutersFacet.sol#L136

Tool
Manual review

Recommendation Mitigation Steps
Add indexed at canonicalId.

#5 Missing amount check

Impact
Being instantiated with wrong configuration the contract will be inoperable.
amount can't be 0.

Proof of Concept
https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/facets/RoutersFacet.sol#L490

Tool
Manual review

Recommendation Mitigation Steps
add

if (_amount == 0) revert()

to the function

#6 Typo
Impact
Misleading the user

Proof of Concept
https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/facets/RoutersFacet.sol#L575

Tool
Manual Review

Recommended Mitigation Steps
Fix it from specicfied to specified.

#7 inclusive condition
Impact
this functions will fail when the end-user expects it to pass through

Proof of Concept
https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/facets/RoutersFacet.sol#L592

Tool
Manual Review

RecommendationMitigation Steps
Conditions routerBalance should be inclusive >= or <= :

#8 Param wrapped must be immutable

Impact
the state wrapped can't be initialize by constructor.

https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L49

Tool Used
Manual Review

Recommended Mitigation Steps
the state must add immutable because in the constructor parameter mention wrapped to initialize. so i suggest to add immutable on it.

#9 transferId must be indexed

Impact
event is missing indexed fields.

Proof of Concept
https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/promise/PromiseRouter.sol#L102

https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/relayer-fee/RelayerFeeRouter.sol#L50

Tool
Manual review

Recommendation Mitigation Steps
Add indexed at transferId.

[jakekidd]

1 invalid: Unused variable is being used in event emit below?
7 invalid: should NOT be inclusive comparison
8 invalid?: needs more explanation, I think? there may or may not be a wrapper contract on a given chain, and that may change, so it can't be immutable

3+4+9 should be 1 issue