code423n4 commented 2 years ago

C4 finding submitted: (risk = 1 (Low Risk)) Missing non-zero address check when setting the contract owner

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Libraries/LibDiamond.sol#L44

Vulnerability details

Impact

setContractOwner(address _newOwner) does not check _newOwner against zero address. Zero address check should be performed especially when changing critical addresses.

Proof of Concept

Tools Used

Manual analysis

Recommended Mitigation Steps

Add zero address check

C4 finding submitted: (risk = 1 (Low Risk)) Setting new contract owner should be a 2-step process

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Libraries/LibDiamond.sol#L44

Vulnerability details

Impact

setContractOwner(address _newOwner) sets the new contract owner directly. If a wrong new owner address is entered, the ownership will be lost and can not be fixed.

Proof of Concept

https://github.com/code-423n4/2022-01-livepeer-findings/issues/143

Tools Used

Manual analysis

Recommended Mitigation Steps

Use a 2-step process where the current owner sets the new owner candidate, and the candidate claims the ownership.

C4 finding submitted: (risk = 1 (Low Risk)) Missing non-zero address check when setting the cBridge router contract

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/CBridgeFacet.sol#L45

Vulnerability details

Impact

initCbridge() does not check _cBridge against zero address.

Proof of Concept

Tools Used

Manual analysis

Recommended Mitigation Steps

Add zero address check

C4 finding submitted: (risk = 0 (Non-critical)) Floating pragma

Lines of code

All the contracts in scope, some of which https://github.com/code-423n4/2022-03-lifinance/blob/main/src/LiFiDiamond.sol#L2 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L2 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/CBridgeFacet.sol#L2

Vulnerability details

Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

Impact

Proof of Concept

https://swcregistry.io/docs/SWC-103

Tools Used

Manual analysis

Recommended Mitigation Steps

Lock the pragma

C4 finding submitted: (risk = 0 (Non-critical)) Unused import

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L6 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L9

Vulnerability details

Impact

LibDiamond.sol is imported (twice) in AnyswapAsset.sol, but it is not used.

Proof of Concept

Tools Used

Manual analysis