dxbjavid commented on code in PR #13404:
URL: https://github.com/apache/trafficserver/pull/13404#discussion_r3612962163
##########
src/tscore/unit_tests/test_Encoding.cc:
##########
@@ -57,6 +57,29 @@ TEST_CASE("Encoding pure escapify url", "[pure_esc_url]")
}
}
+TEST_CASE("Encoding escapify url without a terminator",
"[esc_url_unterminated]")
+{
+ // The source is a counted string, not a C string, so nothing may be read
past len_in.
+ // Sized exactly so that a read past the end is caught by a sanitizer.
+ constexpr std::string_view src{"abcdef"};
+
+ char *unterminated = static_cast<char *>(std::malloc(src.size()));
+ std::memcpy(unterminated, src.data(), src.size());
+
+ char output[128];
+ int output_len;
+
+ REQUIRE(Encoding::pure_escapify_url(nullptr, unterminated, src.size(),
&output_len, output, sizeof(output)) != nullptr);
+ CHECK(output_len == static_cast<int>(src.size()));
+ CHECK(std::string_view(output, output_len) == src);
+
+ REQUIRE(Encoding::escapify_url(nullptr, unterminated, src.size(),
&output_len, output, sizeof(output)) != nullptr);
+ CHECK(output_len == static_cast<int>(src.size()));
+ CHECK(std::string_view(output, output_len) == src);
+
+ std::free(unterminated);
Review Comment:
Done, switched it to a std::vector<char> built straight from the source, so
the exact-size allocation still stands but there's nothing to leak if a REQUIRE
trips. Also added a check that the output is terminated at output_len, since
that's really the other half of what the fix changed.
##########
src/tscore/Encoding.cc:
##########
@@ -77,7 +77,7 @@ escapify_url_common(Arena *arena, char *url, size_t len_in,
int *len_out, char *
static char hex_digit[16] = {'0', '1', '2', '3', '4', '5', '6', '7', '8',
'9', 'A', 'B', 'C', 'D', 'E', 'F'};
- if (!url || (dst && dst_size < len_in)) {
+ if (!url || (dst && dst_size < len_in + 1)) {
Review Comment:
Fair point, len_in is a size_t so a caller passing SIZE_MAX would wrap the
addition and slip past the guard. Rewritten as dst_size <= len_in, which says
the same thing for any sane value and can't overflow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
