Pieces Protocol
First Flight #32
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

[L-3] Redundant `onlyNftOwner` Modifier Usage in `divideNft` Function

koinerdegen

Summary

The divideNft function contains a redundant use of the onlyNftOwner modifier, which is applied twice to the function. This leads to unnecessary gas consumption and redundancy in access control enforcement.

Vulnerability Details

Currently, the function definition is:

@> function divideNft(address nftAddress, uint256 tokenId, uint256 amount) onlyNftOwner(nftAddress, tokenId) onlyNftOwner(nftAddress, tokenId) external {
if(nftAddress == address(0)) { revert TokenDivider__NftAddressIsZero(); }
if(amount == 0) { revert TokenDivider__AmountCantBeZero(); }
ERC20ToGenerateNftFraccion erc20Contract = new ERC20ToGenerateNftFraccion(
string(abi.encodePacked(ERC721(nftAddress).name(), "Fraccion")),
string(abi.encodePacked("F", ERC721(nftAddress).symbol())));
​
erc20Contract.mint(address(this), amount);
address erc20 = address(erc20Contract);
IERC721(nftAddress).safeTransferFrom(msg.sender, address(this), tokenId, "");
if(IERC721(nftAddress).ownerOf(tokenId) == msg.sender) { revert TokenDivider__NftTransferFailed(); }
​
balances[msg.sender][erc20] = amount;
nftToErc20Info[nftAddress] = ERC20Info({erc20Address: erc20, tokenId: tokenId});
erc20ToMintedAmount[erc20] = amount;
erc20ToNft[erc20] = nftAddress;
​
emit NftDivided(nftAddress, amount, erc20);
bool transferSuccess = IERC20(erc20).transfer(msg.sender, amount);
if(!transferSuccess) {
revert TokenDivider__TransferFailed();
}
}

Suggested Improvement:

Remove the redundant modifier application:

- function divideNft(address nftAddress, uint256 tokenId, uint256 amount) onlyNftOwner(nftAddress, tokenId) onlyNftOwner(nftAddress ,tokenId)
+ function divideNft(address nftAddress, uint256 tokenId, uint256 amount) onlyNftOwner(nftAddress, tokenId) external {
+ // Function logic remains the same
+ }

Impact

  • Gas Optimization: Eliminating redundant modifier calls reduces unnecessary gas costs.

  • Code Readability: Improves readability by avoiding duplicate checks.

  • Functionality Assurance: Ensures the same level of security without redundancy.

Tools Used

  • Manual code review

  • Solidity static analysis tools

Recommendations

  1. Remove Duplicate Modifiers: Keep only one instance of onlyNftOwner to avoid redundancy.

  2. Optimize Function Logic: Review modifier placements in other functions to ensure minimal and necessary usage.

  3. Gas Efficiency Consideration: Always review code for redundant operations that could affect gas fees.

Updates

Lead Judging Commences

fishy Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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