QA Report

[code423n4]

[QA-1] _router is not used in repayAavePortalFor function

repayAavePortalFor function says This allows anyone to repay the portal in the adopted asset for a given router and transfer but the argument _router is not used at all.

https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/PortalFacet.sol#L115-L126

---

[QA-2] repayAavePortal function can set underflown value at s.routerBalances[msg.sender][_local]

Random people can call repayAavePortal function and set underflown value at s.routerBalances[msg.sender][_local].

This part decrements balance in the unchecked statement.

https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/PortalFacet.sol#L107-L109

    unchecked {
      s.routerBalances[msg.sender][_local] -= amountIn;
    }

When adopted is local asset and AssetLogic.swapFromLocalAssetIfNeededForExactOut returns (true, _amount, _asset), it may reach to s.routerBalances[msg.sender][_local] -= amountIn.

https://github.com/code-423n4/2022-06-connext/blob/4dd6149748b635f95460d4c3924c7e3fb6716967/contracts/contracts/core/connext/libraries/AssetLogic.sol#L244-L247

As a result, when s.routerBalances[msg.sender][_local] is 0 or small value, it is possible that the underflow happens at s.routerBalances[msg.sender][_local] -= amountIn which results in having giant value at s.routerBalances[msg.sender][_local] which seems not ideal. Removing unchecked from this part can prevent such situations.

[LayneHaber]

QA-1 -- valid.

QA-2 -- invalid -- there are other high severity issues that reference this (and this is a bit more than a QA issue since routers can then withdraw and effectively steal from the contract). See #68 #211