Re: Coverity false positives triggered by gnulib's implementation of base64

[Bruno Haible]

Hi Paul,

> Sorry, I'm still not following. Unless the tainted data is used to
> calculate an array index, there's no problem with Heartbleed and the
> Coverity heuristic should not diagnose a problem.

Yes, IF they were only using an algorithm and no heuristic,
base64_encode would not be flagged as a dangerous consumer of
untrusted input.

But their article
https://www.synopsys.com/blogs/software-security/detecting-heartbleed-with-static-analysis/
says:

  "Program analysis is hard and approximations and trade-offs are
   absolutely mandatory. We’ve found that the best results come from
   a combination of advanced algorithms and knowledge of idioms that
   occur in real-world code."

So they are combining data flow analysis - in order to determine
that the argument of base64_alloc is untrusted data - with a
heuristic - "if a function contains array accesses with indices that
are computed with ntohs calls, we should flag it as dangerous consumer".

> the proposed comment would be wrong as it would pacify
> Coverity without fixing the real bug elsewhere.

The base64_encode function does not make a dangerous array
access (only to the 'b64c' array). And its result is a string,
not an integer, and therefore cannot be used as an array index
either. Therefore adding this comment cannot silence real bugs.

But maybe it will be sufficient to mask all b64c arguments
with '& 0x3f', like you already suggested in the other mail?

Bruno

diff --git a/lib/base64.c b/lib/base64.c
index f3f7298..a00e0f4 100644
--- a/lib/base64.c
+++ b/lib/base64.c
@@ -70,7 +70,7 @@ base64_encode_fast (const char *restrict in, size_t inlen, 
char *restrict out)
 {
   while (inlen)
     {
-      *out++ = b64c[to_uchar (in[0]) >> 2];
+      *out++ = b64c[(to_uchar (in[0]) >> 2) & 0x3f];
       *out++ = b64c[((to_uchar (in[0]) << 4) + (to_uchar (in[1]) >> 4)) & 
0x3f];
       *out++ = b64c[((to_uchar (in[1]) << 2) + (to_uchar (in[2]) >> 6)) & 
0x3f];
       *out++ = b64c[to_uchar (in[2]) & 0x3f];
@@ -103,7 +103,7 @@ base64_encode (const char *restrict in, size_t inlen,
 
   while (inlen && outlen)
     {
-      *out++ = b64c[to_uchar (in[0]) >> 2];
+      *out++ = b64c[(to_uchar (in[0]) >> 2) & 0x3f];
       if (!--outlen)
         break;
       *out++ = b64c[((to_uchar (in[0]) << 4)

Bruno