diff --git a/include/crow/query_string.h b/include/crow/query_string.h index a694b4caf..9fb1ae8c0 100644 --- a/include/crow/query_string.h +++ b/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(*s++); - lnyb = static_cast(*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(*s++); + lnyb = static_cast(*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(*qs++); - lnyb = static_cast(*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(*qs++); + lnyb = static_cast(*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; diff --git a/tests/query_string_tests.cpp b/tests/query_string_tests.cpp index 94e98f863..42209bff9 100644 --- a/tests/query_string_tests.cpp +++ b/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 } \ No newline at end of file