From 0be60063112bb6b1313d7b128824a5e85c1b050f Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Sat, 22 Nov 2025 12:59:42 +0100 Subject: [PATCH] Fix the parser to not accept invalid escapes Only `"\/bfnrtu` are valid after a backslash. --- CHANGES.md | 3 ++ ext/json/ext/parser/parser.c | 53 ++++++++++--------- java/src/json/ext/StringDecoder.java | 21 +++++++- .../fixtures/{pass15.json => fail15.json} | 0 .../fixtures/{pass16.json => fail16.json} | 0 .../fixtures/{pass17.json => fail17.json} | 0 .../fixtures/{pass26.json => fail26.json} | 0 test/json/json_fixtures_test.rb | 2 + test/json/json_parser_test.rb | 4 +- 9 files changed, 53 insertions(+), 30 deletions(-) rename test/json/fixtures/{pass15.json => fail15.json} (100%) rename test/json/fixtures/{pass16.json => fail16.json} (100%) rename test/json/fixtures/{pass17.json => fail17.json} (100%) rename test/json/fixtures/{pass26.json => fail26.json} (100%) diff --git a/CHANGES.md b/CHANGES.md index 83fbaa097..3ebbe1909 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,9 @@ ### Unreleased +* Fixed the parser to no longer ignore invalid escapes in strings. + Only `\"`, `\\`, `\b`, `\f`, `\n`, `\r`, `\t` and `\u` are valid JSON escapes. + ### 2025-11-07 (2.16.0) * Deprecate `JSON::State#[]` and `JSON::State#[]=`. Consider using `JSON::Coder` instead. diff --git a/ext/json/ext/parser/parser.c b/ext/json/ext/parser/parser.c index cb22648db..2f584a51a 100644 --- a/ext/json/ext/parser/parser.c +++ b/ext/json/ext/parser/parser.c @@ -639,44 +639,43 @@ static inline VALUE json_string_fastpath(JSON_ParserState *state, const char *st static VALUE json_string_unescape(JSON_ParserState *state, const char *string, const char *stringEnd, bool is_name, bool intern, bool symbolize) { size_t bufferSize = stringEnd - string; - const char *p = string, *pe = string, *unescape, *bufferStart; + const char *p = string, *pe = string, *bufferStart; char *buffer; - int unescape_len; - char buf[4]; VALUE result = rb_str_buf_new(bufferSize); rb_enc_associate_index(result, utf8_encindex); buffer = RSTRING_PTR(result); bufferStart = buffer; +#define APPEND_CHAR(chr) *buffer++ = chr; p = ++pe; + while (pe < stringEnd && (pe = memchr(pe, '\\', stringEnd - pe))) { - unescape = (char *) "?"; - unescape_len = 1; if (pe > p) { MEMCPY(buffer, p, char, pe - p); buffer += pe - p; } switch (*++pe) { + case '"': + case '/': + p = pe; // nothing to unescape just need to skip the backslash + break; + case '\\': + APPEND_CHAR('\\'); + break; case 'n': - unescape = (char *) "\n"; + APPEND_CHAR('\n'); break; case 'r': - unescape = (char *) "\r"; + APPEND_CHAR('\r'); break; case 't': - unescape = (char *) "\t"; - break; - case '"': - unescape = (char *) "\""; - break; - case '\\': - unescape = (char *) "\\"; + APPEND_CHAR('\t'); break; case 'b': - unescape = (char *) "\b"; + APPEND_CHAR('\b'); break; case 'f': - unescape = (char *) "\f"; + APPEND_CHAR('\f'); break; case 'u': if (pe > stringEnd - 5) { @@ -714,18 +713,23 @@ static VALUE json_string_unescape(JSON_ParserState *state, const char *string, c break; } } - unescape_len = convert_UTF32_to_UTF8(buf, ch); - unescape = buf; + + char buf[4]; + int unescape_len = convert_UTF32_to_UTF8(buf, ch); + MEMCPY(buffer, buf, char, unescape_len); + buffer += unescape_len; + p = ++pe; } break; default: - p = pe; - continue; + if ((unsigned char)*pe < 0x20) { + raise_parse_error_at("invalid ASCII control character in string: %s", state, pe - 1); + } + raise_parse_error_at("invalid escape character in string: %s", state, pe - 1); + break; } - MEMCPY(buffer, unescape, char, unescape_len); - buffer += unescape_len; - p = ++pe; } +#undef APPEND_CHAR if (stringEnd > p) { MEMCPY(buffer, p, char, stringEnd - p); @@ -976,9 +980,6 @@ static inline VALUE json_parse_string(JSON_ParserState *state, JSON_ParserConfig case '\\': { state->cursor++; escaped = true; - if ((unsigned char)*state->cursor < 0x20) { - raise_parse_error("invalid ASCII control character in string: %s", state); - } break; } default: diff --git a/java/src/json/ext/StringDecoder.java b/java/src/json/ext/StringDecoder.java index bf616c1e9..03c78514a 100644 --- a/java/src/json/ext/StringDecoder.java +++ b/java/src/json/ext/StringDecoder.java @@ -69,6 +69,15 @@ private void handleEscapeSequence(ThreadContext context) throws IOException { case 't': append('\t'); break; + case '/': + append('/'); + break; + case '"': + append('"'); + break; + case '\\': + append('\\'); + break; case 'u': ensureMin(context, 4); int cp = readHex(context); @@ -81,8 +90,8 @@ private void handleEscapeSequence(ThreadContext context) throws IOException { writeUtf8Char(cp); } break; - default: // '\\', '"', '/'... - quoteStart(); + default: + throw invalidEscape(context); } } @@ -174,4 +183,12 @@ protected RaiseException invalidUtf8(ThreadContext context) { return Utils.newException(context, Utils.M_PARSER_ERROR, context.runtime.newString(message)); } + + protected RaiseException invalidEscape(ThreadContext context) { + ByteList message = new ByteList( + ByteList.plain("invalid escape character in string: ")); + message.append(src, charStart, srcEnd - charStart); + return Utils.newException(context, Utils.M_PARSER_ERROR, + context.runtime.newString(message)); + } } diff --git a/test/json/fixtures/pass15.json b/test/json/fixtures/fail15.json similarity index 100% rename from test/json/fixtures/pass15.json rename to test/json/fixtures/fail15.json diff --git a/test/json/fixtures/pass16.json b/test/json/fixtures/fail16.json similarity index 100% rename from test/json/fixtures/pass16.json rename to test/json/fixtures/fail16.json diff --git a/test/json/fixtures/pass17.json b/test/json/fixtures/fail17.json similarity index 100% rename from test/json/fixtures/pass17.json rename to test/json/fixtures/fail17.json diff --git a/test/json/fixtures/pass26.json b/test/json/fixtures/fail26.json similarity index 100% rename from test/json/fixtures/pass26.json rename to test/json/fixtures/fail26.json diff --git a/test/json/json_fixtures_test.rb b/test/json/json_fixtures_test.rb index c153ebef7..c0d103793 100644 --- a/test/json/json_fixtures_test.rb +++ b/test/json/json_fixtures_test.rb @@ -10,6 +10,8 @@ class JSONFixturesTest < Test::Unit::TestCase source = File.read(f) define_method("test_#{name}") do assert JSON.parse(source), "Did not pass for fixture '#{File.basename(f)}': #{source.inspect}" + rescue JSON::ParserError + raise "#{File.basename(f)} parsing failure" end end diff --git a/test/json/json_parser_test.rb b/test/json/json_parser_test.rb index a5b476361..3e662bda3 100644 --- a/test/json/json_parser_test.rb +++ b/test/json/json_parser_test.rb @@ -510,8 +510,8 @@ def test_backslash data = ['"'] assert_equal data, parse(json) # - json = '["\\\'"]' - data = ["'"] + json = '["\\/"]' + data = ["/"] assert_equal data, parse(json) json = '["\/"]'