NFT Dealers
First Flight #58
Beginner FriendlyFoundry
100 EXP
View all submissions
Previous Next
Submission Details
Impact: low
Likelihood: medium

`_lockAmount` value not checked in constructor

Author Revealed upon completion

Root + Impact

Description

  • The _lockAmount is taken in constructor and set to the contract lockAmount. The comment says it is suppose to be 20 USDC. But the constructor never check if this value is indeed 20e6 .

  • The deployer could carelessly deploy a lockAmount value that is different than the protocol expected value.

// Root cause in the codebase with @> marks to highlight the relevant section
constructor(
address _owner,
address _usdc,
string memory _collectionName,
string memory _symbol,
string memory _collectionImage,
uint256 _lockAmount
) ERC721(_collectionName, _symbol) {
owner = _owner;
usdc = IERC20(_usdc);
collectionName = _collectionName;
tokenSymbol = _symbol;
collectionImage = _collectionImage;
@> lockAmount = _lockAmount;
// Miss the check on _lockAmount value
}
​

Risk

Likelihood:

  • When the deployer deploy the contract with _lockAmount = 10. This is also acceptable by the constructor.

Impact:

  • The protocol will receive less collateral per minted NFT than expected.

Proof of Concept

The deployer could carelessly deploy a nftdealer contract with no lockAmount.

function testNftDealerHasCorrectCollateralAmt() public {
usdc = new MockUSDC();
// This should fail but will pass as well
nftDealers = new NFTDealers(owner, address(usdc), "NFTDealers", "NFTD", BASE_IMAGE, 0);
​
// This line will fail
assertEq(nftDealers.lockAmount(), 20e6);
}

Recommended Mitigation

We can just assign the 20e6 value to lockAmount in the smart contract and remove its assignment from the constructor.

- uint256 public immutable lockAmount;
+ uint256 public immutable lockAmount = 20e6;
​
constructor(
address _owner,
address _usdc,
string memory _collectionName,
string memory _symbol,
- string memory _collectionImage,
- uint256 _lockAmount
+ string memory _collectionImage
) ERC721(_collectionName, _symbol) {
owner = _owner;
usdc = IERC20(_usdc);
collectionName = _collectionName;
tokenSymbol = _symbol;
collectionImage = _collectionImage;
- lockAmount = _lockAmount;
}
​

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!