BUG/MINOR: log: improper behavior when escaping log data

Patrick Hemmer reported an improper log behavior when using
log-format to escape log data (+E option):
Some bytes were truncated from the output:

- escape_string() function now takes an extra parameter that
  allow the caller to specify input string stop pointer in
  case the input string is not guaranteed to be zero-terminated.
- Minors checks were added into lf_text_len() to make sure dst
  string will not overflow.
- lf_text_len() now makes proper use of escape_string() function.

This should be backported as far as 1.8.

diff --git a/include/haproxy/tools.h b/include/haproxy/tools.h
index 405d9d55b30a700cb23735f539c400ff0220faf7..948c7de8bf1276b5c2156c820eff30dd86603218 100644 (file)
--- a/include/haproxy/tools.h
+++ b/include/haproxy/tools.h
@@ -424,7 +424,8 @@ char *encode_chunk(char *start, char *stop,
 
 /*
  * Tries to prefix characters tagged in the <map> with the <escape>
- * character. The input <string> must be zero-terminated. The result will
+ * character. The input <string> is processed until string_stop
+ * is reached or NULL-byte is encountered. The result will
  * be stored between <start> (included) and <stop> (excluded). This
  * function will always try to terminate the resulting string with a '\0'
  * before <stop>, and will return its position if the conversion
@@ -432,7 +433,7 @@ char *encode_chunk(char *start, char *stop,
  */
 char *escape_string(char *start, char *stop,
                    const char escape, const long *map,
-                   const char *string);
+                   const char *string, const char *string_stop);
 
 /*
  * Tries to prefix characters tagged in the <map> with the <escape>

diff --git a/src/log.c b/src/log.c
index 55baad822fcafc3e0ff9fa0b945c0f50ca3c3855..07b6b75e4582627a7f372781d980b2bb3b4d179e 100644 (file)
--- a/src/log.c
+++ b/src/log.c
@@ -1184,17 +1184,21 @@ char *lf_text_len(char *dst, const char *src, size_t len, size_t size, const str
        }
 
        if (src && len) {
-               if (++len > size)
-                       len = size;
+               /* escape_string and strlcpy2 will both try to add terminating NULL-byte
+                * to dst, so we need to make sure that extra byte will fit into dst
+                * before calling them
+                */
                if (node->options & LOG_OPT_ESC) {
                        char *ret;
 
-                       ret = escape_string(dst, dst + len, '\\', rfc5424_escape_map, src);
+                       ret = escape_string(dst, (dst + size - 1), '\\', rfc5424_escape_map, src, src + len);
                        if (ret == NULL || *ret != '\0')
                                return NULL;
                        len = ret - dst;
                }
                else {
+                       if (++len > size)
+                               len = size;
                        len = strlcpy2(dst, src, len);
                }
 
@@ -1205,6 +1209,7 @@ char *lf_text_len(char *dst, const char *src, size_t len, size_t size, const str
                if (size < 2)
                        return NULL;
                *(dst++) = '-';
+               size -= 1;
        }
 
        if (node->options & LOG_OPT_QUOTE) {

diff --git a/src/tools.c b/src/tools.c
index 34b1ab099cf051e2ce0ad2d44c7fa6b0462a19c5..8771d2dc8f51d9f8804b28a8f7e0c39426b2f3d8 100644 (file)
--- a/src/tools.c
+++ b/src/tools.c
@@ -1975,7 +1975,8 @@ char *encode_chunk(char *start, char *stop,
 
 /*
  * Tries to prefix characters tagged in the <map> with the <escape>
- * character. The input <string> must be zero-terminated. The result will
+ * character. The input <string> is processed until string_stop
+ * is reached or NULL-byte is encountered. The result will
  * be stored between <start> (included) and <stop> (excluded). This
  * function will always try to terminate the resulting string with a '\0'
  * before <stop>, and will return its position if the conversion
@@ -1983,11 +1984,11 @@ char *encode_chunk(char *start, char *stop,
  */
 char *escape_string(char *start, char *stop,
                    const char escape, const long *map,
-                   const char *string)
+                   const char *string, const char *string_stop)
 {
        if (start < stop) {
                stop--; /* reserve one byte for the final '\0' */
-               while (start < stop && *string != '\0') {
+               while (start < stop && string < string_stop && *string != '\0') {
                        if (!ha_bit_test((unsigned char)(*string), map))
                                *start++ = *string;
                        else {