(trafficserver) branch master updated: Don't assume LogAccess::m_client_req_unmapped_url_canon_str is null terminated. (#936) (#11305)

[wkaras]

This is an automated email from the ASF dual-hosted git repository.

wkaras pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 89cdda706d Don't assume LogAccess::m_client_req_unmapped_url_canon_str 
is null terminated. (#936) (#11305)
89cdda706d is described below

commit 89cdda706d991a09df1e887975888099ff041666
Author: Walt Karas <wka...@yahooinc.com>
AuthorDate: Mon Jun 3 18:42:35 2024 -0400

    Don't assume LogAccess::m_client_req_unmapped_url_canon_str is null 
terminated. (#936) (#11305)
    
    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 fd53fd3b07..db386ae575 100644
--- a/src/proxy/logging/LogAccess.cc
+++ b/src/proxy/logging/LogAccess.cc
@@ -1292,8 +1292,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);
   }
 }