From 1ab7ef64d3c0a69f0b98551bcf810739b18acbfc Mon Sep 17 00:00:00 2001 From: Walt Karas Date: Wed, 3 Apr 2024 13:38:31 -0400 Subject: [PATCH] Don't assume LogAccess::m_client_req_unmapped_url_canon_str is null terminated. (#936) In this code: https://github.com/apache/trafficserver/blob/ff100f4f5ce69a00e3f3093f202d5f1aa9bb2ee5/src/proxy/logging/LogAccess.cc#L1591 if the unmapped URL has nothing to escape, m_client_req_unmapped_url_canon_str will retain the value returned by string_get_ref(). string_get_ref() is a wrapper for url_string_get_ref(): https://github.com/apache/trafficserver/blob/ff100f4f5ce69a00e3f3093f202d5f1aa9bb2ee5/include/proxy/hdrs/URL.h#L468 In this case, there is no apparent null termination: https://github.com/apache/trafficserver/blob/ff100f4f5ce69a00e3f3093f202d5f1aa9bb2ee5/src/proxy/hdrs/URL.cc#L631 It looks like this is how the terminal null can be lost on m_ptr_printed_string: https://github.com/apache/trafficserver/blob/ff100f4f5ce69a00e3f3093f202d5f1aa9bb2ee5/src/proxy/hdrs/URL.cc#L360 https://github.com/apache/trafficserver/blob/e0620eb941eab2603b2c230366e0fae5eeb6b57d/include/proxy/hdrs/HdrHeap.h#L255 --- src/proxy/logging/LogAccess.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/proxy/logging/LogAccess.cc b/src/proxy/logging/LogAccess.cc index 1c1e6b65b8c..016b1af81d2 100644 --- a/src/proxy/logging/LogAccess.cc +++ b/src/proxy/logging/LogAccess.cc @@ -1285,8 +1285,9 @@ void LogAccess::set_client_req_unmapped_url_canon(char *buf, int len) { if (buf && m_client_req_unmapped_url_canon_str) { + // m_client_req_unmapped_url_canon_str is not necessarily null terminated. m_client_req_unmapped_url_canon_len = std::min(len, m_client_req_unmapped_url_canon_len); - ink_strlcpy(m_client_req_unmapped_url_canon_str, buf, m_client_req_unmapped_url_canon_len + 1); + memcpy(m_client_req_unmapped_url_canon_str, buf, m_client_req_unmapped_url_canon_len); } }