Re: [PATCH] getopt: pacify gcc -Wanalyzer-null-dereference

[Bruno Haible]

Paul Eggert wrote:
> > Paul Eggert wrote:
> >> * lib/getopt.c (process_long_option): Simplify logic slightly.
> >> This pacifies gcc -flto -Wanalyzer-null-dereference when compiling
> >> GNU tar on x86-64 with gcc 13.2.1 20231205 (Red Hat 13.2.1-6).
> > This appears to trade a false alarm for another false alarm. Namely,
> > now Coverity reports:
> > 
> > 270                           }
> > 271                         if (ambig_set)
> >>>> CID 1574557:  Memory - corruptions  (ARRAY_VS_SINGLETON)
> >>>> Using "ambig_set" as an array.  This might corrupt or misinterpret 
> >>>> adjacent memory locations.
> > 272                           ambig_set[option_index] = 1;
> > 
> > I guess we can ignore it?
> 
> Yes, let's do that. We're already ignoring what must be hundreds of 
> Coverity false positives, and there's little harm in ignoring one more.

Unfortunately, this wasn't a false alarm, but a good alarm.

Namely, I see the 'test-getopt-gnu' test fail (via abort()) on
  - Linux/x86_64 with clang+asan+ubsan,
  - OpenBSD 7.2/sparc64,
and crash (via SIGSEGV) on
  - FreeBSD 14.0/powerpc64.

Debugging it with clang+asan+ubsan, it gets an out-of-bounds access
here:

                    if (ambig_set && ambig_set != &ambig_fallback)
                      ambig_set[option_index] = 1;              // <=== 
getopt.c line 272
                  }

with these local variables:

(gdb) info locals
ambig_set = 0x7ffff5c001a0 ""
ambig_fallback = 0 '\000'
ambig_malloced = 0x0
indfound = 4
nameend = 0x55555576c243 <str+3> ""
namelen = 1
p = 0x555555794b60 <long_options_required+160>
pfound = 0x555555794b40 <long_options_required+128>
n_options = 8
option_index = 5
(gdb) print ambig_set == &ambig_fallback
$5 = 1

This patch fixes it.


2024-01-17  Bruno Haible  <bruno@clisp.org>

        getopt-gnu: Fix out-of-bounds access (regression 2023-12-11).
        * lib/getopt.c (process_long_option): Don't set ambig_set[option_index]
        if ambig_set is &ambig_fallback.

diff --git a/lib/getopt.c b/lib/getopt.c
index 928306304e..f66f119ec5 100644
--- a/lib/getopt.c
+++ b/lib/getopt.c
@@ -268,7 +268,7 @@ process_long_option (int argc, char **argv, const char 
*optstring,
                            ambig_set[indfound] = 1;
                          }
                      }
-                   if (ambig_set)
+                   if (ambig_set && ambig_set != &ambig_fallback)
                      ambig_set[option_index] = 1;
                  }
              }