Fix heap-buffer-overflow in query_string when parsing malformed '%'Fix heap-buffer-overflow in query_string for malformed '%' (#1134)

* Fix heap-buffer-overflow in query_string for malformed '%'

Author: shan-tanu-joshi-3010

include/crow/query_string.h

@@ -64,23 +64,33 @@ inline int qs_strncmp(const char * s, const char * qs, size_t n)
         if ( u1 == '+' ) {  u1 = ' ';  }
         if ( u1 == '%' ) // easier/safer than scanf
         {
-            unyb = static_cast<unsigned char>(*s++);
-            lnyb = static_cast<unsigned char>(*s++);
-            if ( CROW_QS_ISHEX(unyb) && CROW_QS_ISHEX(lnyb) )
+            // Check that next two chars exist and are valid hex before reading
+            if ( CROW_QS_ISHEX(s[0]) && CROW_QS_ISHEX(s[1]) )
+            {
+                unyb = static_cast<unsigned char>(*s++);
+                lnyb = static_cast<unsigned char>(*s++);
                 u1 = (CROW_QS_HEX2DEC(unyb) * 16) + CROW_QS_HEX2DEC(lnyb);
+            }
             else
+            {
                 u1 = '\0';
+            }
         }
 
         if ( u2 == '+' ) {  u2 = ' ';  }
         if ( u2 == '%' ) // easier/safer than scanf
         {
-            unyb = static_cast<unsigned char>(*qs++);
-            lnyb = static_cast<unsigned char>(*qs++);
-            if ( CROW_QS_ISHEX(unyb) && CROW_QS_ISHEX(lnyb) )
+            // Check that next two chars exist and are valid hex before reading
+            if ( CROW_QS_ISHEX(qs[0]) && CROW_QS_ISHEX(qs[1]) )
+            {
+                unyb = static_cast<unsigned char>(*qs++);
+                lnyb = static_cast<unsigned char>(*qs++);
                 u2 = (CROW_QS_HEX2DEC(unyb) * 16) + CROW_QS_HEX2DEC(lnyb);
+            }
             else
+            {
                 u2 = '\0';
+            }
         }
 
         if ( u1 != u2 )
@@ -150,7 +160,9 @@ inline int qs_decode(char * qs)
         if ( qs[j] == '+' ) {  qs[i] = ' ';  }
         else if ( qs[j] == '%' ) // easier/safer than scanf
         {
-            if ( ! CROW_QS_ISHEX(qs[j+1]) || ! CROW_QS_ISHEX(qs[j+2]) )
+            // Check bounds before reading: ensure j+1 and j+2 are within string
+            if ( qs[j+1] == '\0' || qs[j+2] == '\0' ||
+                 ! CROW_QS_ISHEX(qs[j+1]) || ! CROW_QS_ISHEX(qs[j+2]) )
             {
                 qs[i] = '\0';
                 return i;

tests/query_string_tests.cpp

@@ -42,4 +42,37 @@ TEST_CASE( "query string keys" )
             return key == entry.first;});
         REQUIRE(exist == true);
     }
+}
+
+TEST_CASE( "malformed percent encoding - trailing percent" )
+{
+    // URL ending with '%' should not cause heap overflow (Issue #1126)
+    crow::query_string qs1("https://example.com/asdf?%");
+    auto keys1 = qs1.keys();
+    // Should handle gracefully without crashing
+    if (!keys1.empty()) {
+        (void)qs1.get(keys1[0]);
+    }
+
+    // URL with single hex digit after %
+    crow::query_string qs2("https://example.com/?key=%2");
+    auto keys2 = qs2.keys();
+    if (!keys2.empty()) {
+        (void)qs2.get(keys2[0]);
+    }
+
+    // URL with % followed by non-hex characters
+    crow::query_string qs3("https://example.com/?key=%GG");
+    auto keys3 = qs3.keys();
+    if (!keys3.empty()) {
+        (void)qs3.get(keys3[0]);
+    }
+
+    // Valid percent encoding should still work
+    crow::query_string qs4("https://example.com/?key=%20value");
+    auto val = qs4.get("key");
+    REQUIRE(val != nullptr);
+    REQUIRE(std::string(val) == " value");
+
+    REQUIRE(true); // Test passes if no crash/sanitizer error
 }