search

code-423n4 / 2022-06-connext-findings

1 stars 0 forks source link

QA Report #277

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

[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 commented 2 years ago

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