search

code-423n4 / 2022-08-foundation-findings

0 stars 0 forks source link

QA Report #214

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Floating pragma is risky

All contracts in scope

Vulnerability details

The codebase uses floating pragma. All contracts should be compiled with same pragma version. Locking the pragma ensures that contracts do not accidentally get deployed using either an outdated buggy compiler version or a compiler version different from what the code has been tested with.

Recommended Mitigation Steps

Use the same compiler version for all contracts by setting a specific version e.g. 0.8.15  for these contracts.

Misspelling comments

NFTCollectionFactory.sol#L60

Vulnerability details

The comment is wrong, instead of ERC-1165 should be ERC-1167

Recommended Mitigation Steps

- * @dev This creates and initializes an ERC-1165 minimal proxy pointing to a NFT collection contract
+ * @dev This creates and initializes an ERC-1167 minimal proxy pointing to a NFT collection contract

Unused import

NFTDropMarketCore.sol#L5

Vulnerability details

Import is not used in contract

Recommended Mitigation Steps

- import "@openzeppelin/contracts/token/ERC721/IERC721.sol";
HardlyDifficult commented 2 years ago

Use fixed pragma

Disagree. We intentionally use a floating pragma in order to make integrating with contracts easier. Other contract developers are looking to interact with our contracts and they may be on a different version than we use. The pragma selected for our contracts is the minimum required in order to correctly compile and function. This way integration is easier if they lag a few versions behind, or if they use the latest but we don't bump our packages frequently enough, and when we do upgrade versions unless there was a breaking solidity change -- it should just swap in by incrementing our npm package version.

ERC-1165 should be ERC-1167

Agree, will fix.

unused import

Agree, will fix.