code-423n4 / 2022-09-artgobblers-findings

0 stars 0 forks source link

QA Report #123

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

please this is my first audit ;) I'm still trying to learn the ropes, i believe practice makes perfect.

ArtGobblers.revealGobblers(uint256) (src/ArtGobblers.sol#576-685) contains an incorrect shift operation: randomSeed = keccak256(uint256,uint256)(0,32) % 1 << 64 (src/ArtGobblers.sol#674) Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#shift-parameter-mixup

FixedPointMathLib.rpow(uint256,uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#74-160) performs a multiplication on the result of a division: -x = xxRound_rpow_asm_0 / scalar (lib/solmate/src/utils/FixedPointMathLib.sol#131) -zx_rpow_asm_0 = z * x (lib/solmate/src/utils/FixedPointMathLib.sol#136) Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply

Reentrancy in ArtGobblers.mintFromGoo(uint256,bool) (src/ArtGobblers.sol#368-390): External calls:

ArtGobblers.gobble(uint256,address,uint256,bool).owner (src/ArtGobblers.sol#730) shadows:

Owned.setOwner(address).newOwner (lib/solmate/src/auth/Owned.sol#39) lacks a zero-check on :

Reentrancy in ArtGobblers.addGoo(uint256) (src/ArtGobblers.sol#770-776): External calls:

Reentrancy in ArtGobblers.addGoo(uint256) (src/ArtGobblers.sol#770-776): External calls:

ERC20.permit(address,address,uint256,uint256,uint8,bytes32,bytes32) (lib/solmate/src/tokens/ERC20.sol#116-160) uses timestamp for comparisons Dangerous comparisons:

FixedPointMathLib.mulDivDown(uint256,uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#34-51) uses assembly

ERC1155._batchBurn(address,uint256[],uint256[]) (lib/solmate/src/tokens/ERC1155.sol#202-222) is never used and should be removed ERC1155._batchMint(address,uint256[],uint256[],bytes) (lib/solmate/src/tokens/ERC1155.sol#171-200) is never used and should be removed ERC1155._burn(address,uint256,uint256) (lib/solmate/src/tokens/ERC1155.sol#224-232) is never used and should be removed ERC1155._mint(address,uint256,uint256,bytes) (lib/solmate/src/tokens/ERC1155.sol#152-169) is never used and should be removed ERC721._burn(uint256) (lib/solmate/src/tokens/ERC721.sol#172-187) is never used and should be removed ERC721._mint(address,uint256) (lib/solmate/src/tokens/ERC721.sol#157-170) is never used and should be removed ERC721._safeMint(address,uint256) (lib/solmate/src/tokens/ERC721.sol#193-202) is never used and should be removed ERC721._safeMint(address,uint256,bytes) (lib/solmate/src/tokens/ERC721.sol#204-217) is never used and should be removed FixedPointMathLib.divWadDown(uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#22-24) is never used and should be removed FixedPointMathLib.divWadUp(uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#26-28) is never used and should be removed FixedPointMathLib.mulDivUp(uint256,uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#53-72) is never used and should be removed FixedPointMathLib.mulWadUp(uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#18-20) is never used and should be removed FixedPointMathLib.rpow(uint256,uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#74-160) is never used and should be removed FixedPointMathLib.unsafeDiv(uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#238-244) is never used and should be removed FixedPointMathLib.unsafeMod(uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#230-236) is never used and should be removed fromDaysWadUnsafe(int256) (lib/solmate/src/utils/SignedWadMath.sol#28-33) is never used and should be removed wadDiv(int256,int256) (lib/solmate/src/utils/SignedWadMath.sol#67-80) is never used and should be removed Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#dead-code

Pragma version>=0.8.0 (lib/VRGDAs/src/LogisticToLinearVRGDA.sol#2) allows old versions Pragma version>=0.8.0 (lib/VRGDAs/src/LogisticVRGDA.sol#2) allows old versions Pragma version>=0.8.0 (lib/VRGDAs/src/VRGDA.sol#2) allows old versions Pragma version>=0.8.0 (lib/goo-issuance/src/LibGOO.sol#2) allows old versions Pragma version>=0.8.0 (lib/solmate/src/auth/Owned.sol#2) allows old versions Pragma version>=0.8.0 (lib/solmate/src/tokens/ERC1155.sol#2) allows old versions Pragma version>=0.8.0 (lib/solmate/src/tokens/ERC20.sol#2) allows old versions Pragma version>=0.8.0 (lib/solmate/src/tokens/ERC721.sol#2) allows old versions Pragma version>=0.8.0 (lib/solmate/src/utils/FixedPointMathLib.sol#2) allows old versions Pragma version>=0.8.0 (lib/solmate/src/utils/LibString.sol#2) allows old versions Pragma version>=0.8.0 (lib/solmate/src/utils/MerkleProofLib.sol#2) allows old versions Pragma version>=0.8.0 (lib/solmate/src/utils/SignedWadMath.sol#2) allows old versions Pragma version>=0.8.0 (src/ArtGobblers.sol#2) allows old versions Pragma version>=0.8.0 (src/Goo.sol#2) allows old versions Pragma version>=0.8.0 (src/Pages.sol#2) allows old versions Pragma version>=0.8.0 (src/utils/rand/RandProvider.sol#2) allows old versions Pragma version>=0.8.0 (src/utils/token/GobblersERC721.sol#2) allows old versions Pragma version>=0.8.0 (src/utils/token/PagesERC721.sol#2) allows old versions solc-0.8.17 is not recommended for deployment Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-versions-of-solidity

Function ERC20.DOMAIN_SEPARATOR() (lib/solmate/src/tokens/ERC20.sol#162-164) is not in mixedCase Variable ERC20.INITIAL_CHAIN_ID (lib/solmate/src/tokens/ERC20.sol#41) is not in mixedCase Variable ERC20.INITIAL_DOMAIN_SEPARATOR (lib/solmate/src/tokens/ERC20.sol#43) is not in mixedCase Variable ERC721._ownerOf (lib/solmate/src/tokens/ERC721.sol#31) is not in mixedCase Variable ERC721._balanceOf (lib/solmate/src/tokens/ERC721.sol#33) is not in mixedCase Variable ArtGobblers.UNREVEALED_URI (src/ArtGobblers.sol#136) is not in mixedCase Variable ArtGobblers.BASE_URI (src/ArtGobblers.sol#139) is not in mixedCase Variable Pages.BASE_URI (src/Pages.sol#96) is not in mixedCase Variable PagesERC721._ownerOf (src/utils/token/PagesERC721.sol#50) is not in mixedCase Variable PagesERC721._balanceOf (src/utils/token/PagesERC721.sol#52) is not in mixedCase Variable PagesERC721._isApprovedForAll (src/utils/token/PagesERC721.sol#70) is not in mixedCase Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#conformance-to-solidity-naming-conventions

Variable ArtGobblers.UNREVEALED_URI (src/ArtGobblers.sol#136) is too similar to ArtGobblers.constructor(bytes32,uint256,Goo,Pages,address,address,RandProvider,string,string)._unrevealedUri (src/ArtGobblers.sol#299) Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#variable-names-are-too-similar

FixedPointMathLib.sqrt(uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#166-228) uses literals with too many digits:

ERC1155 (lib/solmate/src/tokens/ERC1155.sol#6-233) does not implement functions:

computeGOOBalance(uint256,uint256,uint256) should be declared external:

Goo.constructor(address,address)._artGobblers (src/Goo.sol#82) lacks a zero-check on :

ERC20.permit(address,address,uint256,uint256,uint8,bytes32,bytes32) (lib/solmate/src/tokens/ERC20.sol#116-160) uses timestamp for comparisons Dangerous comparisons:

Pragma version>=0.8.0 (lib/solmate/src/tokens/ERC20.sol#2) allows old versions Pragma version>=0.8.0 (src/Goo.sol#2) allows old versions solc-0.8.17 is not recommended for deployment Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-versions-of-solidity

Function ERC20.DOMAIN_SEPARATOR() (lib/solmate/src/tokens/ERC20.sol#162-164) is not in mixedCase Variable ERC20.INITIAL_CHAIN_ID (lib/solmate/src/tokens/ERC20.sol#41) is not in mixedCase Variable ERC20.INITIAL_DOMAIN_SEPARATOR (lib/solmate/src/tokens/ERC20.sol#43) is not in mixedCase Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#conformance-to-solidity-naming-conventions

approve(address,uint256) should be declared external:

ArtGobblers.revealGobblers(uint256) (src/ArtGobblers.sol#576-685) contains an incorrect shift operation: randomSeed = keccak256(uint256,uint256)(0,32) % 1 << 64 (src/ArtGobblers.sol#674) Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#shift-parameter-mixup

FixedPointMathLib.rpow(uint256,uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#74-160) performs a multiplication on the result of a division: -x = xxRound_rpow_asm_0 / scalar (lib/solmate/src/utils/FixedPointMathLib.sol#131) -zx_rpow_asm_0 = z * x (lib/solmate/src/utils/FixedPointMathLib.sol#136) Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply

Reentrancy in ArtGobblers.mintFromGoo(uint256,bool) (src/ArtGobblers.sol#368-390): External calls:

ArtGobblers.gobble(uint256,address,uint256,bool).owner (src/ArtGobblers.sol#730) shadows:

Owned.setOwner(address).newOwner (lib/solmate/src/auth/Owned.sol#39) lacks a zero-check on :

Reentrancy in ArtGobblers.addGoo(uint256) (src/ArtGobblers.sol#770-776): External calls:

Reentrancy in ArtGobblers.addGoo(uint256) (src/ArtGobblers.sol#770-776): External calls:

ERC20.permit(address,address,uint256,uint256,uint8,bytes32,bytes32) (lib/solmate/src/tokens/ERC20.sol#116-160) uses timestamp for comparisons Dangerous comparisons:

FixedPointMathLib.mulDivDown(uint256,uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#34-51) uses assembly

ERC1155._batchBurn(address,uint256[],uint256[]) (lib/solmate/src/tokens/ERC1155.sol#202-222) is never used and should be removed ERC1155._batchMint(address,uint256[],uint256[],bytes) (lib/solmate/src/tokens/ERC1155.sol#171-200) is never used and should be removed ERC1155._burn(address,uint256,uint256) (lib/solmate/src/tokens/ERC1155.sol#224-232) is never used and should be removed ERC1155._mint(address,uint256,uint256,bytes) (lib/solmate/src/tokens/ERC1155.sol#152-169) is never used and should be removed ERC721._burn(uint256) (lib/solmate/src/tokens/ERC721.sol#172-187) is never used and should be removed ERC721._mint(address,uint256) (lib/solmate/src/tokens/ERC721.sol#157-170) is never used and should be removed ERC721._safeMint(address,uint256) (lib/solmate/src/tokens/ERC721.sol#193-202) is never used and should be removed ERC721._safeMint(address,uint256,bytes) (lib/solmate/src/tokens/ERC721.sol#204-217) is never used and should be removed FixedPointMathLib.divWadDown(uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#22-24) is never used and should be removed FixedPointMathLib.divWadUp(uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#26-28) is never used and should be removed FixedPointMathLib.mulDivUp(uint256,uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#53-72) is never used and should be removed FixedPointMathLib.mulWadUp(uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#18-20) is never used and should be removed FixedPointMathLib.rpow(uint256,uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#74-160) is never used and should be removed FixedPointMathLib.unsafeDiv(uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#238-244) is never used and should be removed FixedPointMathLib.unsafeMod(uint256,uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#230-236) is never used and should be removed fromDaysWadUnsafe(int256) (lib/solmate/src/utils/SignedWadMath.sol#28-33) is never used and should be removed wadDiv(int256,int256) (lib/solmate/src/utils/SignedWadMath.sol#67-80) is never used and should be removed Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#dead-code

Pragma version>=0.8.0 (lib/VRGDAs/src/LogisticToLinearVRGDA.sol#2) allows old versions Pragma version>=0.8.0 (lib/VRGDAs/src/LogisticVRGDA.sol#2) allows old versions Pragma version>=0.8.0 (lib/VRGDAs/src/VRGDA.sol#2) allows old versions Pragma version>=0.8.0 (lib/goo-issuance/src/LibGOO.sol#2) allows old versions Pragma version>=0.8.0 (lib/solmate/src/auth/Owned.sol#2) allows old versions Pragma version>=0.8.0 (lib/solmate/src/tokens/ERC1155.sol#2) allows old versions Pragma version>=0.8.0 (lib/solmate/src/tokens/ERC20.sol#2) allows old versions Pragma version>=0.8.0 (lib/solmate/src/tokens/ERC721.sol#2) allows old versions Pragma version>=0.8.0 (lib/solmate/src/utils/FixedPointMathLib.sol#2) allows old versions Pragma version>=0.8.0 (lib/solmate/src/utils/LibString.sol#2) allows old versions Pragma version>=0.8.0 (lib/solmate/src/utils/MerkleProofLib.sol#2) allows old versions Pragma version>=0.8.0 (lib/solmate/src/utils/SignedWadMath.sol#2) allows old versions Pragma version>=0.8.0 (src/ArtGobblers.sol#2) allows old versions Pragma version>=0.8.0 (src/Goo.sol#2) allows old versions Pragma version>=0.8.0 (src/Pages.sol#2) allows old versions Pragma version>=0.8.0 (src/utils/rand/RandProvider.sol#2) allows old versions Pragma version>=0.8.0 (src/utils/token/GobblersERC721.sol#2) allows old versions Pragma version>=0.8.0 (src/utils/token/PagesERC721.sol#2) allows old versions solc-0.8.17 is not recommended for deployment Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-versions-of-solidity

Function ERC20.DOMAIN_SEPARATOR() (lib/solmate/src/tokens/ERC20.sol#162-164) is not in mixedCase Variable ERC20.INITIAL_CHAIN_ID (lib/solmate/src/tokens/ERC20.sol#41) is not in mixedCase Variable ERC20.INITIAL_DOMAIN_SEPARATOR (lib/solmate/src/tokens/ERC20.sol#43) is not in mixedCase Variable ERC721._ownerOf (lib/solmate/src/tokens/ERC721.sol#31) is not in mixedCase Variable ERC721._balanceOf (lib/solmate/src/tokens/ERC721.sol#33) is not in mixedCase Variable ArtGobblers.UNREVEALED_URI (src/ArtGobblers.sol#136) is not in mixedCase Variable ArtGobblers.BASE_URI (src/ArtGobblers.sol#139) is not in mixedCase Variable Pages.BASE_URI (src/Pages.sol#96) is not in mixedCase Variable PagesERC721._ownerOf (src/utils/token/PagesERC721.sol#50) is not in mixedCase Variable PagesERC721._balanceOf (src/utils/token/PagesERC721.sol#52) is not in mixedCase Variable PagesERC721._isApprovedForAll (src/utils/token/PagesERC721.sol#70) is not in mixedCase Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#conformance-to-solidity-naming-conventions

Variable ArtGobblers.UNREVEALED_URI (src/ArtGobblers.sol#136) is too similar to ArtGobblers.constructor(bytes32,uint256,Goo,Pages,address,address,RandProvider,string,string)._unrevealedUri (src/ArtGobblers.sol#299) Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#variable-names-are-too-similar

FixedPointMathLib.sqrt(uint256) (lib/solmate/src/utils/FixedPointMathLib.sol#166-228) uses literals with too many digits:

ERC1155 (lib/solmate/src/tokens/ERC1155.sol#6-233) does not implement functions:

computeGOOBalance(uint256,uint256,uint256) should be declared external:

GalloDaSballo commented 1 year ago

You can't just paste Slither output and expect anyone to take your work seriously I recommend you use Slither as tool to inspect the code and then enrich it with your observations

90% of this report is incorrect

The formatting is non-existent

To be honest I may have spent more time writing this reply than you took to write this report, anytime that's the case, expect to have your reports close as invalid.

I hope you'll do better next time