search

code-423n4 / 2022-03-lifinance-findings

6 stars 4 forks source link

QA Report #174

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Pin Solidity Version

pin Solidity version to lastest 0.8.12

Set dexWhitelist in _removeDex function

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/DexManagerFacet.sol#L69

s.dexWhitelist[_dex] = false;

to _removeDex private function

Use of transfer might fail in the future

transfer() only forward 2300 gas which may break when gas cost change in a future ETH upgrade see: https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/WithdrawFacet.sol#L31

Make setContractOwner a 2 step process

make ownership transfer a 2 step process to prevent accidentally buring the ownership

initCbridge can be called multiple times

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/CBridgeFacet.sol#L42

initNXTP can be called multiple times

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/NXTPFacet.sol#L33

initHop can be called multiple times

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/HopFacet.sol#L40

Lack events on critical action

e.g. addDex https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/DexManagerFacet.sol#L17

H3xept commented 2 years ago

Re deprecated transfer()

Duplicate of #14

H3xept commented 2 years ago

Re solidity version

We are going to hold off with the pragma changes for now.

H3xept commented 2 years ago

Re 2 step ownership transfer

Duplicate of #143

gzeoneth commented 2 years ago

Report submitted by contest judge.