Re: [PATCH] Widen STRNCATF temporary buffer

[Mark Mentovai]

Patches submitted for review should be in Gerrit, our code review system (https://chromium-review.googlesource.com/q/project:breakpad%252Fbreakpad). Use “git cl upload” to put your patch into that system, and send your review request from there. git-cl is part of the Chromium depot_tools, and the process generally follows Chromium’s contribution process (https://chromium.googlesource.com/chromium/src/+/master/docs/contributing.md).

On Thu, Jan 9, 2020 at 5:34 AM Marc Gonzalez <marc.w....@free.fr> wrote:

From: Marc Gonzalez <marc.w....@free.fr>
Date: Thu, 19 Dec 2019 11:36:26 +0100

The temporary buffer used in STRNCATF is too small for a few callers,
which could lead to truncated output in some situations.

Signed-off-by: Marc Gonzalez <marc.w....@free.fr>
---
No copyright on this trivial patch.

FWIW, here are the warnings generated by gcc:

src/third_party/libdisasm/x86_format.c: In function 'format_operand_xml.isra.3':
src/third_party/libdisasm/x86_format.c:837:5: warning: '%04X' directive output truncated writing 4 bytes into a region of size 1 [-Wformat-truncation=]
     "\t\t<absolute_address segment=\"0x%04" PRIX16 "\"",
     ^
src/third_party/libdisasm/x86_format.c:34:38: note: in definition of macro 'STRNCATF'
         snprintf( _tmp, sizeof _tmp, fmt, data );   \
                                      ^~~
src/third_party/libdisasm/x86_format.c:837:40: note: format string is defined here
     "\t\t<absolute_address segment=\"0x%04" PRIX16 "\"",
src/third_party/libdisasm/x86_format.c:837:5: note: directive argument in the range [0, 65535]
     "\t\t<absolute_address segment=\"0x%04" PRIX16 "\"",
     ^
src/third_party/libdisasm/x86_format.c:34:38: note: in definition of macro 'STRNCATF'
         snprintf( _tmp, sizeof _tmp, fmt, data );   \
                                      ^~~
In file included from /usr/include/stdio.h:862:0,
                 from src/third_party/libdisasm/x86_format.c:1:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:64:10: note: '__builtin___snprintf_chk' output 37 bytes into a destination of size 32
   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        __bos (__s), __fmt, __va_arg_pack ());
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/third_party/libdisasm/x86_format.c: In function 'x86_format_insn':
src/third_party/libdisasm/x86_format.c:1141:24: warning: '|' directive output may be truncated writing 1 byte into a region of size between 0 and 31 [-Wformat-truncation=]
         STRNCATF( buf, "|%s|", insn->prefix_string             , len );
                        ^
src/third_party/libdisasm/x86_format.c:34:38: note: in definition of macro 'STRNCATF'
         snprintf( _tmp, sizeof _tmp, fmt, data );   \
                                      ^~~
In file included from /usr/include/stdio.h:862:0,
                 from src/third_party/libdisasm/x86_format.c:1:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:64:10: note: '__builtin___snprintf_chk' output between 3 and 34 bytes into a destination of size 32
   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        __bos (__s), __fmt, __va_arg_pack ());
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/third_party/libdisasm/x86_format.c:1181:24: warning: '%s' directive output may be truncated writing up to 31 bytes into a region of size 22 [-Wformat-truncation=]
         STRNCATF( buf, "\" string=\"%s\"/>\n", insn->prefix_string, len );
                        ^
src/third_party/libdisasm/x86_format.c:34:38: note: in definition of macro 'STRNCATF'
         snprintf( _tmp, sizeof _tmp, fmt, data );   \
                                      ^~~
In file included from /usr/include/stdio.h:862:0,
                 from src/third_party/libdisasm/x86_format.c:1:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:64:10: note: '__builtin___snprintf_chk' output between 15 and 46 bytes into a destination of size 32
   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        __bos (__s), __fmt, __va_arg_pack ());
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
---
 src/third_party/libdisasm/x86_format.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/third_party/libdisasm/x86_format.c b/src/third_party/libdisasm/x86_format.c
index 0ec960dc..8e83247e 100644
--- a/src/third_party/libdisasm/x86_format.c
+++ b/src/third_party/libdisasm/x86_format.c
@@ -29,7 +29,7 @@
 } while( 0 )

 #define STRNCATF( buf, fmt, data, len ) do {        \
-        char _tmp[MAX_OP_STRING];                   \
+        char _tmp[64];                              \
                                                     \
         snprintf( _tmp, sizeof _tmp, fmt, data );   \
         STRNCAT( buf, _tmp, len );                  \
--
2.17.1