Hi Kamil,
> There are 3 important state-transitions in the data-flow analysis:
>
> (1) obtaining data from untrusted source
> (2) sanitizing the data (checking bounds etc.)
> (3) unsafe use of untrusted data
>
> gnulib's base64_encode() as seen by Coverity Analysis represents (3)
> because its implementation uses byte swaps. This is a heuristic that
> is not always correct, so false positives may happen. If you ask why
> byte swaps are checked, I believe it was inspired by Heartbleed:
>
> https://www.synopsys.com/blogs/software-security/detecting-heartbleed-with-static-analysis/
>
> The inline annotation that I proposed as a patch gives Coverity a hint
> that gnulib's implementation of base64_encode() can safely process data
> from untrusted sources. The annotation is specific to the implementation
> of the function, not to users of the function.
Ah, thanks for explaining. Now I agree: base64_encode produces the
warning because of the (x << n) | (y >> m) expression patterns that
resemble a byte swap. It would do so also for any other program that
contains a base64_encode invocation with untrusted input as argument.
> > Does it need to be done in the source code at all?
>
> Yes, in case of gnulib this is the only sensible option.
> ...
> Yes, various tools exist to waive false positives. The problem is that
> instances of these tools do not share data with each other in the universe.
> Consequently, developers have to repeatedly review these false positives
> and waive them in each single instance of these tools. And even worse with
> gnulib because these false positives are usually not matched across different
> project that bundle gnulib, even if you have a single instance of the waiving
> tool in your organisation.
So, I propose to bite the bullet, but at least put a reasonable comment.
2019-05-09 Kamil Dudka <kdu...@redhat.com>
Bruno Haible <br...@clisp.org>
base64: Avoid false positive warning from Coverity.
* lib/base64.c (base64_encode): Add special comment for Coverity.
diff --git a/lib/base64.c b/lib/base64.c
index f3f7298..80428bb 100644
--- a/lib/base64.c
+++ b/lib/base64.c
@@ -84,6 +84,11 @@ base64_encode_fast (const char *restrict in, size_t inlen,
char *restrict out)
If OUTLEN is less than BASE64_LENGTH(INLEN), write as many bytes as
possible. If OUTLEN is larger than BASE64_LENGTH(INLEN), also zero
terminate the output buffer. */
+/* Tell Coverity that this function works fine when called with IN
+ pointing to untrusted input. By default, Coverity, seeing the value
+ shift expressions below, thinks that it is dangerous to call this
+ function with untrusted input.
+ coverity[-tainted_data_sink: arg-0] */
void
base64_encode (const char *restrict in, size_t inlen,
char *restrict out, size_t outlen)
