Subject: bug#78880: od Heap-buffer overflow

From: Pádraig Brady <[email-masked]>
Date: Sun, 29 Jun 2025 20:59:14 +0100

On 29/06/2025 13:17, Pádraig Brady wrote:
> On 29/06/2025 05:25, Paul Eggert wrote:
> 
>   > od: omit some duplicate code
>   > On x86-64 (for example) print_long, print_long_long, and
>   > print_intmax all behave identically, so give GCC enough info so
>   > that it generates code for just one of these functions.
>   > * src/od.c (enum size_spec): Arrange for enum values to
>   > be the same if they represent types that behave the same.
>   > (width_bytes, ISPEC_TO_FORMAT, decode_one_format):
>   > Match the enum size_spec changes.
> 
> The patch above causes a warning on some systems:
> 
>     src/od.c: In function 'decode_one_format':
>     src/od.c:900:28: error: duplicated 'if' condition [-Werror=duplicated-cond]
>       900 |         else if (size_spec == FLOAT_HALF)
>           |                  ~~~~~~~~~~^~~~~~~~~~~~~
>     src/od.c:895:28: note: previously used here
>       895 |         else if (size_spec == FLOAT_SINGLE)
>           |                  ~~~~~~~~~~^~~~~~~~~~~~~~~
> 
> If we want the compiler to just apply Dead Code Elimination here,
> then it may be best to push/pop ignoring that warning ?

FYI, related to this I see our coverity instance is complaining about deadcode
in the following block, which might be suppressed with this change:

diff --git a/src/od.c b/src/od.cindex 50319aa83..c621ef227 100644
--- a/src/od.c
+++ b/src/od.c
@@ -802,12 +802,17 @@ decode_one_format (char const *s_orig, char const *s, char const **next,
           and prefer INTMAX to LONG_LONG.  */
        print_function
          = (size_spec == INT ? print_int
+           /* coverity[DEADCODE] */
             : size_spec == SHORT ? (fmt == SIGNED_DECIMAL
                                     ? print_s_short : print_short)
+           /* coverity[DEADCODE] */
             : size_spec == CHAR ? (fmt == SIGNED_DECIMAL
                                    ? print_s_char : print_char)
+           /* coverity[DEADCODE] */
             : size_spec == LONG ? print_long
+           /* coverity[DEADCODE] */
             : size_spec == INTMAX ? print_intmax
+           /* coverity[DEADCODE] */
             : size_spec == LONG_LONG ? print_long_long
             : (affirm (false), nullptr));
        break;

That's a bit messy to me though (the existing comment should
suffice for humans to know the deadcode is intentional).
So I've manually suppressed that error instance in our coverity instance.

cheers,
Padraig