bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Bruno Haible
Subject: Re: Coverity false positives triggered by gnulib's implementation of base64
Date: Wed, 08 May 2019 10:15:37 +0200
User-agent: KMail/5.1.3 (Linux/4.4.0-145-generic; KDE/5.18.0; x86_64; ; )

Hi Kamil,

> Coverity Analysis 2019.03 incorrectly marks the input argument 
> of base64_encode(), and conseuqnetly base64_encode_alloc(), as
> tainted_data_sink because it sees byte-level operations on the input.
> 
> It triggered the following false positives in the cryptsetup project:
> 
> Error: TAINTED_SCALAR:
> lib/luks2/luks2_digest_pbkdf2.c:117: tainted_data_argument: Calling function 
> "crypt_random_get" taints argument "salt".
> lib/luks2/luks2_digest_pbkdf2.c:157: tainted_data: Passing tainted variable 
> "salt" to a tainted sink.
> 
> Error: TAINTED_SCALAR:
> lib/luks2/luks2_keyslot_luks2.c:445: tainted_data_argument: Calling function 
> "crypt_random_get" taints argument "salt".
> lib/luks2/luks2_keyslot_luks2.c:448: tainted_data: Passing tainted variable 
> "salt" to a tainted sink.
> 
> 
> ... but it can affect other gnulib-based projects, too.  Would it be
> possible to apply the following one-line patch on gnulib source code
> to suppress this class of false positives in gnulib-based projects?
> 
> https://gitlab.com/cryptsetup/cryptsetup/commit/75b2610e

When I read the description of "tainted data" [1] and of the
recommendations how to deal with such warnings [2], it is clear that
the warning/error is about the global data flow. Therefore it seems
inappropriate to me to put annotations about the global data flow
into gnulib (which is shared among multiple packages).

Therefore, what I would suggest is that you create an inline function
that merely invokes base64_encode_alloc, use it in line 157 of [3],
and put your annotation on it.

Does it need to be done in the source code at all? Some Coverity tools
have a UI that allows the developers to mark the findings as "false
positive" or as "handled" without touching the source code, and these
marks persist across modifications of the source code. Don't you have
this possibility for this Coverity tool?

Bruno

[1] https://community.synopsys.com/s/article/Tainted-data-in-Coverity
[2] 
https://stackoverflow.com/questions/24772247/how-to-handle-coverity-error-tainted-scalar-in-fread
[3] https://fossies.org/linux/cryptsetup/lib/luks2/luks2_digest_pbkdf2.c




reply via email to

[Prev in Thread] Current Thread [Next in Thread]