code-423n4 / 2022-07-swivel-findings

0 stars 1 forks source link

QA Report #104

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

code commented out it is best practice to remove commented out code

https://github.com/code-423n4/2022-07-swivel/blob/fbf94f87994d91dce75c605a1822ec6d6d7e9e74/Swivel/Hash.sol#L27-L35 https://github.com/code-423n4/2022-07-swivel/blob/fbf94f87994d91dce75c605a1822ec6d6d7e9e74/Swivel/Hash.sol#L38-L52

typos

instead of tranferring use : transferring https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Swivel/Interfaces.sol#L125 instead of ddress use : address https://github.com/code-423n4/2022-07-swivel/blob/fbf94f87994d91dce75c605a1822ec6d6d7e9e74/Marketplace/Compounding.sol#L40

instead of a unlicensed spdx-License

use like mit one or which ever one is best your your project but you should have one as best practice https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Swivel/Sig.sol#L1 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Swivel/Interfaces.sol#L1 https://github.com/code-423n4/2022-07-swivel/blob/fbf94f87994d91dce75c605a1822ec6d6d7e9e74/Swivel/Hash.sol#L1

no address 0 check

https://github.com/code-423n4/2022-07-swivel/blob/fbf94f87994d91dce75c605a1822ec6d6d7e9e74/Marketplace/Compounding.sol#L61 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Creator/Creator.sol#L48 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L36-L41 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L164

if address is not a contract then the getting the underlying token will revert.

c=can equal any address so maybe = tx.origan

    if (p == uint8(Protocols.Compound)) {

            return ICompoundToken(c).underlying();

https://github.com/code-423n4/2022-07-swivel/blob/fbf94f87994d91dce75c605a1822ec6d6d7e9e74/Marketplace/Compounding.sol#L61

if 4626 erc standord token dosnt have correct or weird convertToAssets the function will revert or it will fail silenly

https://github.com/code-423n4/2022-07-swivel/blob/fbf94f87994d91dce75c605a1822ec6d6d7e9e74/Marketplace/Compounding.sol#L83

no emiting events after importent functions

https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Creator/Creator.sol#L48 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Creator/Creator.sol#L55 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L164 https://github.com/code-423n4/2022-07-swivel/blob/67c6900222cc4045d7fe2227a1ea73e0251374ed/Creator/ZcToken.sol#L139-L156

make a 2 step for making the new admin

because instead of giving the admin rights away the new admin should approve his role https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Creator/Creator.sol#L48 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Creator/Creator.sol#L55 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L164

make a or m variables into real words to make it make sence.

https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Creator/Creator.sol#L48 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Creator/Creator.sol#L56 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L164 https://github.com/code-423n4/2022-07-swivel/blob/67c6900222cc4045d7fe2227a1ea73e0251374ed/Creator/ZcToken.sol#L139-L156

instead of a : newadmin instead of m: newmarketplace instead of c: newmaturityRate https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L275 instead of o: user

make sure an admin cant get comprimed and increase the maturityRate causing the malious admin to gain funds

https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L164 https://github.com/code-423n4/2022-07-swivel/blob/67c6900222cc4045d7fe2227a1ea73e0251374ed/Creator/ZcToken.sol#L139-L156

missing natspec comments

this function is missing the @parm holder comment https://github.com/code-423n4/2022-07-swivel/blob/67c6900222cc4045d7fe2227a1ea73e0251374ed/Creator/ZcToken.sol#L98

robrobbins commented 2 years ago

going to address license for this