OpenSSL static analysis, was: De Raadt + FBSD + OpenSSH + hole?

[Charles Swiger]

Hi--

On Apr 24, 2014, at 3:58 AM, Ben Laurie <benl at freebsd.org> wrote:
[ ... ]
>> It's worth noting that even if you believe that (e.g.) the clang static analyzer isn't properly doing liveness analysis and misjudging whether there's a dead assignment (writing to a variable which is never read), the clang compiler will be using the same analysis when doing dead-code elimination and common-subexpression elimination and such while optimizing.
> 
> I think this is not true. I could be wrong, but I've actually worked
> on clang static analysis and I think it is an entirely separate
> system. Certainly there's no guarantee that a static analysis result
> will be reflected in the output of the compiler.

You appear to be disagreeing with something which was almost-- but not quite-- what I said.  :-)

scan-build invokes a wrapper called ccc-analyzer (for C code; c++-analyzer for C++), which they interpose around the compiler such as clang or even gcc.  The docs are informative:

 --use-cc=[compiler path]

   scan-build analyzes a project by interposing a "fake compiler", which
   executes a real compiler for compilation and the static analyzer for analysis.
   Because of the current implementation of interposition, scan-build does not
   know what compiler your project normally uses.  Instead, it simply overrides
   the CC environment variable, and guesses your default compiler.

   In the future, this interposition mechanism to be improved, but if you need
   scan-build to use a specific compiler for *compilation* then you can use
   this option to specify a path to that compiler.

 --use-analyzer [Xcode|path to clang]
 --use-analyzer=[Xcode|path to clang]

   scan-build uses the 'clang' executable relative to itself for static
   analysis. One can override this behavior with this option by using the
   'clang' packaged with Xcode (on OS X) or from the PATH.

...and to pick a specific example from the end of the openssl-1.0.1g build+scan:

~/WorkAreas/llvm/tools/clang/tools/scan-build/ccc-analyzer -DMONOLITH -I.. -I../include  -DOPENSSL_THREADS -D_REENTRANT -DDSO_DLFCN -DHAVE_DLFCN_H -arch x86_64 -O3 -DL_ENDIAN -Wall -DOPENSSL_IA32_SSE2 -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_MONT5 -DOPENSSL_BN_ASM_GF2m -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DMD5_ASM -DAES_ASM -DVPAES_ASM -DBSAES_ASM -DWHIRLPOOL_ASM -DGHASH_ASM   -c -o engine.o engine.c
engine.c:114:3: warning: Value stored to 'l' is never read
                l += 2;         /* ", " */
                ^    ~
1 warning generated.

That's from clang.  You can get the same output by invoking clang directly as:

clang -DMONOLITH -I.. -I../include  -DOPENSSL_THREADS -D_REENTRANT -DDSO_DLFCN -DHAVE_DLFCN_H -arch x86_64 -O3 -DL_ENDIAN -Wall -DOPENSSL_IA32_SSE2 -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_MONT5 -DOPENSSL_BN_ASM_GF2m -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DMD5_ASM -DAES_ASM -DVPAES_ASM -DBSAES_ASM -DWHIRLPOOL_ASM -DGHASH_ASM -Wall --analyze -c -o engine.o engine.c
engine.c:114:3: warning: Value stored to 'l' is never read
                l += 2;         /* ", " */
                ^    ~
1 warning generated.

If you check the file, openssl-1.0.1g/apps/engine.c around line 114, you'll see it was quite right:

static int append_buf(char **buf, const char *s, int *size, int step)
        {
        int l = strlen(s);

        if (*buf == NULL)
                {
                *size = step;
                *buf = OPENSSL_malloc(*size);
                if (*buf == NULL)
                        return 0;
                **buf = '\0';
                }

        if (**buf != '\0')
                l += 2;         /* ", " */

        if (strlen(*buf) + strlen(s) >= (unsigned int)*size)
                {
                *size += step;
                *buf = OPENSSL_realloc(*buf, *size);
                }

        if (*buf == NULL)
                return 0;

        if (**buf != '\0')
                BUF_strlcat(*buf, ", ", *size);
        BUF_strlcat(*buf, s, *size);

        return 1;
        }

Ewww.

Regards,
-- 
-Chuck