Fix JSON Injection Vulnerability in License Terms Handling (storyprot…

…ocol#24)
kingster-will · Jan 31, 2025 · ce3cbc9 · ce3cbc9
1 parent 802cba8
commit ce3cbc9
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 0 deletions.
diff --git a/contracts/lib/PILicenseTemplateErrors.sol b/contracts/lib/PILicenseTemplateErrors.sol
@@ -57,4 +57,7 @@ library PILicenseTemplateErrors {
 
     /// @notice Zero address provided for Royalty Module at initialization.
     error PILicenseTemplate__ZeroRoyaltyModule();
+
+    /// @notice The URI field in PILTerms contains invalid double quote characters(").
+    error PILicenseTemplate__PILTermsURIContainsDoubleQuote(string uri);
 }
diff --git a/contracts/modules/licensing/PILicenseTemplate.sol b/contracts/modules/licensing/PILicenseTemplate.sol
@@ -104,6 +104,10 @@ contract PILicenseTemplate is
             revert PILicenseTemplateErrors.PILicenseTemplate__RoyaltyPolicyRequiresCurrencyToken();
         }
 
+        if (_containsDoubleQuote(terms.uri)) {
+            revert PILicenseTemplateErrors.PILicenseTemplate__PILTermsURIContainsDoubleQuote(terms.uri);
+        }
+
         _verifyCommercialUse(terms);
         _verifyDerivatives(terms);
 
@@ -439,6 +443,23 @@ contract PILicenseTemplate is
     function _exists(uint256 licenseTermsId) internal view returns (bool) {
         return licenseTermsId <= _getPILicenseTemplateStorage().licenseTermsCounter;
     }
+
+    /// @dev Checks if the URI contains double quotes.
+    /// @param uri The URI string to validate.
+    /// @return returns true if the URI contains at least one double quote, false otherwise.
+    function _containsDoubleQuote(string memory uri) internal pure returns (bool) {
+        bytes memory uriBytes = bytes(uri);
+        // solhint-disable-next-line quotes
+        bytes1 doubleQuote = bytes('"')[0];
+
+        for (uint256 i = 0; i < uriBytes.length; i++) {
+            if (uriBytes[i] == doubleQuote) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     ////////////////////////////////////////////////////////////////////////////
     //                         Upgrades related                               //
     ////////////////////////////////////////////////////////////////////////////

diff --git a/test/foundry/modules/licensing/PILicenseTemplate.t.sol b/test/foundry/modules/licensing/PILicenseTemplate.t.sol
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: BUSL-1.1
 pragma solidity 0.8.26;
 
+// solhint-disable quotes
+
 // external
 import { Strings } from "@openzeppelin/contracts/utils/Strings.sol";
 
@@ -678,6 +680,79 @@ contract PILicenseTemplateTest is BaseTest {
         );
     }
 
+    function test_PILicenseTemplate_registerLicenseTerms_revert_PILTermsURIContainsDoubleQuote() public {
+        string memory maliciousUri1 = string.concat(
+            '"}], ',
+            '"name" : "", ',
+            '"description" : "", ',
+            '"external_url" : "", ',
+            '"image" : "", ',
+            '"attributes" : [], ',
+            '"old_attributes" : [{"ok" : ""}'
+        );
+
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                PILicenseTemplateErrors.PILicenseTemplate__PILTermsURIContainsDoubleQuote.selector,
+                maliciousUri1
+            )
+        );
+        pilTemplate.registerLicenseTerms(
+            PILTerms({
+                transferable: true,
+                royaltyPolicy: address(0),
+                defaultMintingFee: 0,
+                expiration: 0,
+                commercialUse: false,
+                commercialAttribution: false,
+                commercializerChecker: address(0),
+                commercializerCheckerData: "",
+                commercialRevShare: 0,
+                commercialRevCeiling: 0,
+                derivativesAllowed: false,
+                derivativesAttribution: false,
+                derivativesApproval: false,
+                derivativesReciprocal: false,
+                derivativeRevCeiling: 0,
+                currency: address(0),
+                uri: maliciousUri1
+            })
+        );
+
+        string memory maliciousUri2 = string.concat(
+            unicode"https://github.com/storyprotocol/protocol-core",
+            '"', // The malicious quote character
+            "/blob/main/PIL_Beta_Final_2024_02.pdf"
+        );
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                PILicenseTemplateErrors.PILicenseTemplate__PILTermsURIContainsDoubleQuote.selector,
+                maliciousUri2
+            )
+        );
+        pilTemplate.registerLicenseTerms(
+            PILTerms({
+                transferable: true,
+                royaltyPolicy: address(0),
+                defaultMintingFee: 0,
+                expiration: 0,
+                commercialUse: false,
+                commercialAttribution: false,
+                commercializerChecker: address(0),
+                commercializerCheckerData: "",
+                commercialRevShare: 0,
+                commercialRevCeiling: 0,
+                derivativesAllowed: false,
+                derivativesAttribution: false,
+                derivativesApproval: false,
+                derivativesReciprocal: false,
+                derivativeRevCeiling: 0,
+                currency: address(0),
+                uri: maliciousUri2
+            })
+        );
+    }
+
     function onERC721Received(address, address, uint256, bytes memory) public pure returns (bytes4) {
         return this.onERC721Received.selector;
     }