From 491caf4d7bf546aec0767a89e234119359f63e9c Mon Sep 17 00:00:00 2001 From: ThisIsFineTM <184989147+ThisIsFineTM@users.noreply.github.com> Date: Mon, 13 Jan 2025 16:27:10 -0500 Subject: [PATCH] Fix #439 for '%XX' at the end of a link going off the end of the buffer. --- src/tools.cpp | 20 +++++++++++++------- test/tools-test.cpp | 8 ++++---- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/tools.cpp b/src/tools.cpp index 1459eb60..dfced863 100644 --- a/src/tools.cpp +++ b/src/tools.cpp @@ -404,23 +404,24 @@ std::string normalize_link(const std::string& input, const std::string& baseUrl) output.reserve(baseUrl.size() + input.size() + 1); bool check_rel = false; - const char* p = input.c_str(); + auto p = input.cbegin(); if ( *(p) == '/') { // This is an absolute url. p++; } else { //This is a relative url, use base url output = baseUrl; - if (!output.empty() && output.back() != '/') + if (!output.empty() && output.back() != '/') { output += '/'; + } check_rel = true; } //URL Decoding. - while (*p) + while (p < input.cend()) { if ( check_rel ) { - if (strncmp(p, "../", 3) == 0) { + if (std::strncmp(&*p, "../", 3) == 0) { // We must go "up" // Remove the '/' at the end of output. output.resize(output.size()-1); @@ -432,7 +433,7 @@ std::string normalize_link(const std::string& input, const std::string& baseUrl) check_rel = false; continue; } - if (strncmp(p, "./", 2) == 0) { + if (std::strncmp(&*p, "./", 2) == 0) { // We must simply skip this part // Simply move after the ".". p += 2; @@ -449,8 +450,13 @@ std::string normalize_link(const std::string& input, const std::string& baseUrl) if ( *p == '%') { - char ch; - sscanf(p+1, "%2hhx", &ch); + if( (p+3) >= input.cend()){ + // if the %XX token would go off the end of the string, just break + break; + } + // hhx only officially supports hex unsigned char + unsigned char ch = 0; + std::sscanf(&*(p+1), "%2hhx", &ch); output += ch; p += 3; continue; diff --git a/test/tools-test.cpp b/test/tools-test.cpp index 9254c700..13b8eced 100644 --- a/test/tools-test.cpp +++ b/test/tools-test.cpp @@ -257,8 +257,8 @@ TEST(tools, normalize_link) // #439: normalized link reading off end of buffer // small-string-opt sizes, so sanitizers and valgrind don't pick this up - EXPECT_EQ(normalize_link("%", "/"), "/"); - EXPECT_EQ(normalize_link("%1", ""), ""); + ASSERT_EQ(normalize_link("%", "/"), "/"); + ASSERT_EQ(normalize_link("%1", ""), ""); // ../test/tools-test.cpp:260: Failure // Expected equality of these values: @@ -274,8 +274,8 @@ TEST(tools, normalize_link) // test outside of small-string-opt // valgrind will pick up on the error in this one - EXPECT_EQ(normalize_link("qrstuvwxyz%", "/abcdefghijklmnop"), "/abcdefghijklmnop/qrstuvwxyz"); - EXPECT_EQ(normalize_link("qrstuvwxyz%1", "/abcdefghijklmnop"), "/abcdefghijklmnop/qrstuvwxyz"); + ASSERT_EQ(normalize_link("qrstuvwxyz%", "/abcdefghijklmnop"), "/abcdefghijklmnop/qrstuvwxyz"); + ASSERT_EQ(normalize_link("qrstuvwxyz%1", "/abcdefghijklmnop"), "/abcdefghijklmnop/qrstuvwxyz"); } TEST(tools, addler32)