Fix four out-of-bounds reads in the lexer and tag URI validation (#547)

Found by running `tests/fuzz_test` under `libFuzzer` and `AddressSanitizer`.
Each of these reads past the end of the input buffer and reproduces on
develop with an input of at most 35 bytes:

* "!<" - `extract_tag_name()` advances past `m_end_itr` before the scanning
  loop's end test can fire.
* "!!a%41" - `uri_encoding::validate()` advances the iterator twice per
  percent escape, so an escape at the end of the URI moves it past end.
  The character after a valid `%XX` was also never validated.
* "%TAG !  " - `scan_tag_directive()` dereferences `m_cur_itr` after
  `skip_white_spaces()` may have consumed the rest of the buffer.
* "\"\\\\\\\"" - the backslash counting loop in
  `determine_double_quoted_scalar_range()` never examines the token's first
  character, so an odd run of backslashes is read as even and the scalar
  parser is handed a token one byte short.

Two results change for inputs that did not crash: "!!a%41^" is now
rejected and "!!%41" is now accepted.

Fixes #546

Author: sndth

include/fkYAML/detail/encodings/uri_encoding.hpp

@@ -37,6 +37,11 @@ class uri_encoding {
                     return false;
                 }
 
+                // validate_octets() advances `current` past the last octet it consumed. Without
+                // moving it back, the loop's own ++current skips the character which follows the
+                // escape sequence, and, when the escape ends the sequence, moves `current` one
+                // past `end` so that the loop condition never holds and reads out of bounds.
+                --current;
                 continue;
             }
 

include/fkYAML/detail/input/lexical_analyzer.hpp

@@ -554,6 +554,11 @@ class lexical_analyzer {
         // extract a tag prefix.
         //
 
+        // skip_white_spaces() above may have consumed the rest of the input buffer.
+        if FK_YAML_UNLIKELY (m_cur_itr == m_end_itr) {
+            emit_error("invalid TAG directive is found.");
+        }
+
         m_token_begin_itr = m_cur_itr;
         const char* p_tag_prefix_begin = m_cur_itr;
         switch (*m_cur_itr) {
@@ -684,7 +689,9 @@ class lexical_analyzer {
         case '<':
             // Verbatim tags (!<TAG>)
             is_verbatim = true;
-            ++m_cur_itr;
+            if FK_YAML_UNLIKELY (++m_cur_itr == m_end_itr) {
+                emit_error("verbatim tag (!<TAG>) must be ended with \'>\'.");
+            }
             break;
         default:
             // Either local tags (!suffix) or named handles (!tag!suffix)
@@ -816,12 +823,18 @@ class lexical_analyzer {
                 // * even number of backslashes -> double quotation mark IS NOT escaped (e.g., "\\"")
                 uint32_t backslash_counts = 0;
                 const char* p = m_token_begin_itr + (pos - 1);
-                do {
-                    if (*p-- != '\\') {
+                for (;;) {
+                    if (*p != '\\') {
                         break;
                     }
                     ++backslash_counts;
-                } while (p != m_token_begin_itr);
+                    if (p == m_token_begin_itr) {
+                        // the first character of the token has just been counted. Stopping here
+                        // also keeps `p` from being decremented past the beginning of the token.
+                        break;
+                    }
+                    --p;
+                }
                 is_closed = ((backslash_counts & 1u) == 0); // true: even, false: odd
             }
 

single_include/fkYAML/node.hpp

@@ -1310,6 +1310,11 @@ class uri_encoding {
                     return false;
                 }
 
+                // validate_octets() advances `current` past the last octet it consumed. Without
+                // moving it back, the loop's own ++current skips the character which follows the
+                // escape sequence, and, when the escape ends the sequence, moves `current` one
+                // past `end` so that the loop condition never holds and reads out of bounds.
+                --current;
                 continue;
             }
 
@@ -3782,6 +3787,11 @@ class lexical_analyzer {
         // extract a tag prefix.
         //
 
+        // skip_white_spaces() above may have consumed the rest of the input buffer.
+        if FK_YAML_UNLIKELY (m_cur_itr == m_end_itr) {
+            emit_error("invalid TAG directive is found.");
+        }
+
         m_token_begin_itr = m_cur_itr;
         const char* p_tag_prefix_begin = m_cur_itr;
         switch (*m_cur_itr) {
@@ -3912,7 +3922,9 @@ class lexical_analyzer {
         case '<':
             // Verbatim tags (!<TAG>)
             is_verbatim = true;
-            ++m_cur_itr;
+            if FK_YAML_UNLIKELY (++m_cur_itr == m_end_itr) {
+                emit_error("verbatim tag (!<TAG>) must be ended with \'>\'.");
+            }
             break;
         default:
             // Either local tags (!suffix) or named handles (!tag!suffix)
@@ -4044,12 +4056,18 @@ class lexical_analyzer {
                 // * even number of backslashes -> double quotation mark IS NOT escaped (e.g., "\\"")
                 uint32_t backslash_counts = 0;
                 const char* p = m_token_begin_itr + (pos - 1);
-                do {
-                    if (*p-- != '\\') {
+                for (;;) {
+                    if (*p != '\\') {
                         break;
                     }
                     ++backslash_counts;
-                } while (p != m_token_begin_itr);
+                    if (p == m_token_begin_itr) {
+                        // the first character of the token has just been counted. Stopping here
+                        // also keeps `p` from being decremented past the beginning of the token.
+                        break;
+                    }
+                    --p;
+                }
                 is_closed = ((backslash_counts & 1u) == 0); // true: even, false: odd
             }
 

tests/unit_test/test_fuzz_regression.cpp

@@ -118,4 +118,39 @@ TEST_CASE("FuzzRegression") {
         p_end = p_begin + sizeof(input);
         REQUIRE_THROWS_AS(root = fkyaml::node::deserialize(p_begin, p_end), fkyaml::invalid_encoding);
     }
+
+    SUBCASE("verbatim tag which ends the input buffer") {
+        const char input[] = "!<";
+        p_begin = input;
+        p_end = input + sizeof(input) - 1;
+        REQUIRE_THROWS_AS(root = fkyaml::node::deserialize(p_begin, p_end), fkyaml::parse_error);
+    }
+
+    SUBCASE("TAG directive without a tag prefix") {
+        const char input[] = "%TAG !  ";
+        p_begin = input;
+        p_end = input + sizeof(input) - 1;
+        REQUIRE_THROWS_AS(root = fkyaml::node::deserialize(p_begin, p_end), fkyaml::parse_error);
+    }
+
+    SUBCASE("percent escape at the end of a tag URI") {
+        const char input[] = "!!a%41";
+        p_begin = input;
+        p_end = input + sizeof(input) - 1;
+        REQUIRE_NOTHROW(root = fkyaml::node::deserialize(p_begin, p_end));
+    }
+
+    SUBCASE("invalid URI character right after a percent escape") {
+        const char input[] = "!!a%41^";
+        p_begin = input;
+        p_end = input + sizeof(input) - 1;
+        REQUIRE_THROWS_AS(root = fkyaml::node::deserialize(p_begin, p_end), fkyaml::parse_error);
+    }
+
+    SUBCASE("double quoted scalar closed by an escaped quotation mark") {
+        const char input[] = "\"\\\\\\\"";
+        p_begin = input;
+        p_end = input + sizeof(input) - 1;
+        REQUIRE_THROWS_AS(root = fkyaml::node::deserialize(p_begin, p_end), fkyaml::parse_error);
+    }
 }