Skip to content

Add Foundry fuzz tests for Strings.escapeJSON #6466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Sandra-Cai wants to merge 4 commits into
base: master
Choose a base branch
from
+50 −0
Open

Add Foundry fuzz tests for Strings.escapeJSON #6466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ecccaa4
Add Foundry fuzz tests for Strings.escapeJSON
Sandra-Cai Apr 7, 2026
a3c324e
Validate hex digits in \u00XX escape sequences
Sandra-Cai Apr 7, 2026
7e91977
Fix prettier formatting
Sandra-Cai Apr 7, 2026
d14ffea
Add changeset for escapeJSON fuzz tests
Sandra-Cai Apr 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/quiet-moons-teach.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': patch
---

Add Foundry fuzz tests for `Strings.escapeJSON`.
45 changes: 45 additions & 0 deletions test/utils/Strings.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,49 @@ contract StringsTest is Test {
(bool success, ) = input.tryParseAddress(begin, begin + 40);
assertFalse(success);
}

function testEscapeJSONLength(string memory input) external pure {
assertGe(bytes(input.escapeJSON()).length, bytes(input).length);
}

// Validates the output of escapeJSON is well-formed JSON string content:
// - no unescaped control characters (U+0000 to U+001F)
// - no unescaped double quotes
// - every backslash begins a valid escape sequence (\b \t \n \f \r \\ \" or \u00XX)
function testEscapeJSON(string memory input) external pure {
bytes memory escaped = bytes(input.escapeJSON());

for (uint256 i = 0; i < escaped.length; i++) {
uint8 c = uint8(escaped[i]);
assertGe(c, 0x20);

if (c == 0x5c) {
assertLt(i + 1, escaped.length);
uint8 next = uint8(escaped[++i]);
if (next == 0x75) {
// \u00XX
assertLt(i + 4, escaped.length);
assertEq(uint8(escaped[i + 1]), 0x30);
assertEq(uint8(escaped[i + 2]), 0x30);
uint8 hi = uint8(escaped[i + 3]);
uint8 lo = uint8(escaped[i + 4]);
assertTrue((hi >= 0x30 && hi <= 0x39) || (hi >= 0x41 && hi <= 0x46) || (hi >= 0x61 && hi <= 0x66));
assertTrue((lo >= 0x30 && lo <= 0x39) || (lo >= 0x41 && lo <= 0x46) || (lo >= 0x61 && lo <= 0x66));
i += 4;
} else {
Comment on lines +69 to +79
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Validate the final XX bytes in \u00XX are hex digits.

Line 72-74 verifies \u00 shape but does not verify that the last two bytes are [0-9a-fA-F]. This weakens the “well-formed JSON escape” property and can miss malformed unicode escapes.

Suggested patch 
                 if (next == 0x75) {
                     // \u00XX
                     assertLt(i + 4, escaped.length);
                     assertEq(uint8(escaped[i + 1]), 0x30);
                     assertEq(uint8(escaped[i + 2]), 0x30);
+                    uint8 hi = uint8(escaped[i + 3]);
+                    uint8 lo = uint8(escaped[i + 4]);
+                    assertTrue(
+                        (hi >= 0x30 && hi <= 0x39) || (hi >= 0x41 && hi <= 0x46) || (hi >= 0x61 && hi <= 0x66)
+                    );
+                    assertTrue(
+                        (lo >= 0x30 && lo <= 0x39) || (lo >= 0x41 && lo <= 0x46) || (lo >= 0x61 && lo <= 0x66)
+                    );
                     i += 4;
                 } else {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (next == 0x75) {
// \u00XX
assertLt(i + 4, escaped.length);
assertEq(uint8(escaped[i + 1]), 0x30);
assertEq(uint8(escaped[i + 2]), 0x30);
i += 4;
} else {
if (next == 0x75) {
// \u00XX
assertLt(i + 4, escaped.length);
assertEq(uint8(escaped[i + 1]), 0x30);
assertEq(uint8(escaped[i + 2]), 0x30);
uint8 hi = uint8(escaped[i + 3]);
uint8 lo = uint8(escaped[i + 4]);
assertTrue(
(hi >= 0x30 && hi <= 0x39) || (hi >= 0x41 && hi <= 0x46) || (hi >= 0x61 && hi <= 0x66)
);
assertTrue(
(lo >= 0x30 && lo <= 0x39) || (lo >= 0x41 && lo <= 0x46) || (lo >= 0x61 && lo <= 0x66)
);
i += 4;
} else {
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test/utils/Strings.t.sol` around lines 69 - 75, The test currently checks for
the "\u00" prefix but doesn't validate the two hex digits that follow; update
the branch where next == 0x75 to (1) ensure the buffer has both hex nibbles
present (use assertLt(i + 5, escaped.length) or equivalent) and (2) add
assertions that escaped[i + 3] and escaped[i + 4] are valid hex characters by
testing each byte against ranges '0'-'9', 'A'-'F', and 'a'-'f' (use uint8
comparisons), keeping references to the same variables (escaped, i, next) so the
test fails on malformed Unicode escapes.

assertTrue(
next == 0x62 || // \b
next == 0x74 || // \t
next == 0x6e || // \n
next == 0x66 || // \f
next == 0x72 || // \r
next == 0x5c || // \\
next == 0x22 // \"
);
}
} else {
assertTrue(c != 0x22);
}
}
}
}