implementing suggestions from Egis audit review (#960)

* refactor: index admin in AllowToHook event

* feat: include dash in alphanumeric check

* build: update precompiles

* docs: update natspec

* refactor: rename isAlphanumericWithSpaces to IsAllowedCharacter

* test: fixes

---------

Co-authored-by: Paul Razvan Berg <prberg@proton.me>

src/SablierV2NFTDescriptor.sol

@@ -320,22 +320,23 @@ contract SablierV2NFTDescriptor is ISablierV2NFTDescriptor {
         return string.concat("Sablier V2 ", sablierModel, " #", streamId);
     }
 
-    /// @notice Checks whether the provided string contains only alphanumeric characters and spaces.
+    /// @notice Checks whether the provided string contains only alphanumeric characters, spaces, and dashes.
     /// @dev Note that this returns true for empty strings.
-    function isAlphanumericWithSpaces(string memory str) internal pure returns (bool) {
+    function isAllowedCharacter(string memory str) internal pure returns (bool) {
         // Convert the string to bytes to iterate over its characters.
         bytes memory b = bytes(str);
 
         uint256 length = b.length;
         for (uint256 i = 0; i < length; ++i) {
             bytes1 char = b[i];
 
-            // Check if it's a space or an alphanumeric character.
+            // Check if it's a space, dash, or an alphanumeric character.
             bool isSpace = char == 0x20; // space
+            bool isDash = char == 0x2D; // dash
             bool isDigit = char >= 0x30 && char <= 0x39; // 0-9
             bool isUppercaseLetter = char >= 0x41 && char <= 0x5A; // A-Z
             bool isLowercaseLetter = char >= 0x61 && char <= 0x7A; // a-z
-            if (!(isSpace || isDigit || isUppercaseLetter || isLowercaseLetter)) {
+            if (!(isSpace || isDash || isDigit || isUppercaseLetter || isLowercaseLetter)) {
                 return false;
             }
         }
@@ -381,12 +382,12 @@ contract SablierV2NFTDescriptor is ISablierV2NFTDescriptor {
 
         string memory symbol = abi.decode(returnData, (string));
 
-        // Check if the symbol is too long or contains non-alphanumeric characters, this measure helps mitigate
-        // potential security threats from malicious assets injecting scripts in the symbol string.
+        // Check if the symbol is too long or contains disallowed characters. This measure helps mitigate potential
+        // security threats from malicious assets injecting scripts in the symbol string.
         if (bytes(symbol).length > 30) {
             return "Long Symbol";
         } else {
-            if (!isAlphanumericWithSpaces(symbol)) {
+            if (!isAllowedCharacter(symbol)) {
                 return "Unsupported Symbol";
             }
             return symbol;

src/interfaces/ISablierV2Lockup.sol

@@ -24,7 +24,7 @@ interface ISablierV2Lockup is
     /// @notice Emitted when the admin allows a new recipient contract to hook to Sablier.
     /// @param admin The address of the current contract admin.
     /// @param recipient The address of the recipient contract put on the allowlist.
-    event AllowToHook(address admin, address recipient);
+    event AllowToHook(address indexed admin, address recipient);
 
     /// @notice Emitted when a stream is canceled.
     /// @param streamId The ID of the stream.

test/integration/concrete/nft-descriptor/is-allowed-character/IsAllowedCharacter.t.sol

@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity >=0.8.22 <0.9.0;
+
+import { NFTDescriptor_Integration_Shared_Test } from "../../../shared/nft-descriptor/NFTDescriptor.t.sol";
+
+contract IsAllowedCharacter_Integration_Concrete_Test is NFTDescriptor_Integration_Shared_Test {
+    function test_IsAllowedCharacter_EmptyString() external view {
+        string memory symbol = "";
+        bool result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertTrue(result, "isAllowedCharacter");
+    }
+
+    modifier whenNotEmptyString() {
+        _;
+    }
+
+    function test_IsAllowedCharacter_ContainsUnsupportedCharacters() external view whenNotEmptyString {
+        string memory symbol = "<foo/>";
+        bool result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertFalse(result, "isAllowedCharacter");
+
+        symbol = "foo/";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertFalse(result, "isAllowedCharacter");
+
+        symbol = "foo\\";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertFalse(result, "isAllowedCharacter");
+
+        symbol = "foo%";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertFalse(result, "isAllowedCharacter");
+
+        symbol = "foo&";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertFalse(result, "isAllowedCharacter");
+
+        symbol = "foo(";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertFalse(result, "isAllowedCharacter");
+
+        symbol = "foo)";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertFalse(result, "isAllowedCharacter");
+
+        symbol = "foo\"";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertFalse(result, "isAllowedCharacter");
+
+        symbol = "foo'";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertFalse(result, "isAllowedCharacter");
+
+        symbol = "foo`";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertFalse(result, "isAllowedCharacter");
+
+        symbol = "foo;";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertFalse(result, "isAllowedCharacter");
+
+        symbol = "foo%20"; // URL-encoded empty space
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertFalse(result, "isAllowedCharacter");
+    }
+
+    modifier whenOnlySupportedCharacters() {
+        _;
+    }
+
+    function test_IsAllowedCharacter() external view whenNotEmptyString whenOnlySupportedCharacters {
+        string memory symbol = "foo";
+        bool result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertTrue(result, "isAllowedCharacter");
+
+        symbol = "Foo";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertTrue(result, "isAllowedCharacter");
+
+        symbol = "Foo ";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertTrue(result, "isAllowedCharacter");
+
+        symbol = "Foo Bar";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertTrue(result, "isAllowedCharacter");
+
+        symbol = "Bar-Foo";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertTrue(result, "isAllowedCharacter");
+
+        symbol = "  ";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertTrue(result, "isAllowedCharacter");
+
+        symbol = "foo01234";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertTrue(result, "isAllowedCharacter");
+
+        symbol = "123456789";
+        result = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertTrue(result, "isAllowedCharacter");
+    }
+}

test/integration/concrete/nft-descriptor/is-alphanumeric-with-spaces/isAlphanumericWithSpaces.tree → test/integration/concrete/nft-descriptor/is-allowed-character/IsAllowedCharacter.tree

@@ -1,4 +1,4 @@
-isAlphanumericWithSpaces.t.sol
+isAllowedCharacter.t.sol
 ├── when the symbol is an empty string
 │  └── it should return true
 └── when the symbol is not an empty string

test/integration/fuzz/nft-descriptor/isAlphanumericWithSpaces.t.sol → test/integration/fuzz/nft-descriptor/isAllowedCharacter.t.sol

@@ -3,8 +3,9 @@ pragma solidity >=0.8.22 <0.9.0;
 
 import { NFTDescriptor_Integration_Shared_Test } from "../../shared/nft-descriptor/NFTDescriptor.t.sol";
 
-contract IsAlphanumericWithSpaces_Integration_Fuzz_Test is NFTDescriptor_Integration_Shared_Test {
+contract IsAllowedCharacter_Integration_Fuzz_Test is NFTDescriptor_Integration_Shared_Test {
     bytes1 internal constant SPACE = 0x20; // ASCII 32
+    bytes1 internal constant DASH = 0x2D; // ASCII 45
     bytes1 internal constant ZERO = 0x30; // ASCII 48
     bytes1 internal constant NINE = 0x39; // ASCII 57
     bytes1 internal constant A = 0x41; // ASCII 65
@@ -21,7 +22,7 @@ contract IsAlphanumericWithSpaces_Integration_Fuzz_Test is NFTDescriptor_Integra
     /// - String with only alphanumerical characters
     /// - String with only non-alphanumerical characters
     /// - String with both alphanumerical and non-alphanumerical characters
-    function testFuzz_IsAlphanumericWithSpaces(string memory symbol) external view whenNotEmptyString {
+    function testFuzz_IsAllowedCharacter(string memory symbol) external view whenNotEmptyString {
         bytes memory b = bytes(symbol);
         uint256 length = b.length;
         bool expectedResult = true;
@@ -32,15 +33,16 @@ contract IsAlphanumericWithSpaces_Integration_Fuzz_Test is NFTDescriptor_Integra
                 break;
             }
         }
-        bool actualResult = nftDescriptorMock.isAlphanumericWithSpaces_(symbol);
-        assertEq(actualResult, expectedResult, "isAlphanumericWithSpaces");
+        bool actualResult = nftDescriptorMock.isAllowedCharacter_(symbol);
+        assertEq(actualResult, expectedResult, "isAllowedCharacter");
     }
 
     function isAlphanumericOrSpaceChar(bytes1 char) internal pure returns (bool) {
         bool isSpace = char == SPACE;
+        bool isDash = char == DASH;
         bool isDigit = char >= ZERO && char <= NINE;
         bool isUppercaseLetter = char >= A && char <= Z;
         bool isLowercaseLetter = char >= a && char <= z;
-        return isSpace || isDigit || isUppercaseLetter || isLowercaseLetter;
+        return isSpace || isDash || isDigit || isUppercaseLetter || isLowercaseLetter;
     }
 }

test/mocks/NFTDescriptorMock.sol

@@ -78,8 +78,8 @@ contract NFTDescriptorMock is SablierV2NFTDescriptor {
         return SVGElements.hourglass(status);
     }
 
-    function isAlphanumericWithSpaces_(string memory symbol) external pure returns (bool) {
-        return isAlphanumericWithSpaces(symbol);
+    function isAllowedCharacter_(string memory symbol) external pure returns (bool) {
+        return isAllowedCharacter(symbol);
     }
 
     function mapSymbol_(IERC721Metadata nft) external view returns (string memory) {

test/utils/Events.sol

@@ -32,7 +32,7 @@ abstract contract Events {
                                  SABLIER-V2-LOCKUP
     //////////////////////////////////////////////////////////////////////////*/
 
-    event AllowToHook(address admin, address recipient);
+    event AllowToHook(address indexed admin, address recipient);
 
     event CancelLockupStream(
         uint256 streamId,