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

2019-05-10 Thread Paul Eggert
On 5/10/19 4:32 AM, Kamil Dudka wrote: I do not think it is a good idea to change a piece of working code to make a static analysis false positives magically disappear. I was thinking of making a change only if it makes the code a bit better even ignoring whether Coverity is used. Surely we

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

2019-05-10 Thread Kamil Dudka
On Friday, May 10, 2019 4:11:45 PM CEST Bruno Haible wrote: > Kamil Dudka wrote: > > Thanks! This also helps to suppress the false positives on cryptsetup > > with Coverity Static Analysis version 2019.03. > > Good! Since this is the approach that Paul prefers, I'm pushing this one: > > >

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

2019-05-10 Thread Bruno Haible
Kamil Dudka wrote: > Thanks! This also helps to suppress the false positives on cryptsetup > with Coverity Static Analysis version 2019.03. Good! Since this is the approach that Paul prefers, I'm pushing this one: 2019-05-10 Bruno Haible base64: Avoid false positive warning from

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

2019-05-10 Thread Kamil Dudka
On Friday, May 10, 2019 12:13:32 AM CEST Bruno Haible wrote: > 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

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

2019-05-10 Thread Kamil Dudka
On Friday, May 10, 2019 1:34:55 PM CEST Florian Weimer wrote: > * Kamil Dudka: > >> For example, how do you know that the reports are false positives and not > >> true positives? > > > > I think it was obvious from my previous explanation: > > > > (1) You need to check (by manual review) that

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

2019-05-10 Thread Kamil Dudka
On Thursday, May 9, 2019 10:35:18 PM CEST Bruno Haible wrote: > 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 > > > >

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

2019-05-10 Thread Florian Weimer
* Kamil Dudka: >> For example, how do you know that the reports are false positives and not >> true positives? > > I think it was obvious from my previous explanation: > > (1) You need to check (by manual review) that the source of data is really > untrusted. > > (2) You need to check (by manual

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

2019-05-10 Thread Kamil Dudka
On Thursday, May 9, 2019 9:14:40 PM CEST Paul Eggert wrote: > On 5/7/19 7:22 AM, Kamil Dudka wrote: > > 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

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

2019-05-09 Thread Paul Eggert
On 5/9/19 3:13 PM, Bruno Haible wrote: > 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

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

2019-05-09 Thread 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

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

2019-05-09 Thread Paul Eggert
On 5/9/19 1:35 PM, Bruno Haible wrote: >> https://www.synopsys.com/blogs/software-security/detecting-heartbleed-with-static-analysis/ >> > 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

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

2019-05-09 Thread Bruno Haible
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

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

2019-05-09 Thread Paul Eggert
On 5/7/19 7:22 AM, Kamil Dudka wrote: > 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". >

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

2019-05-09 Thread Kamil Dudka
Hi Bruno, On Wednesday, May 8, 2019 10:15:37 AM CEST Bruno Haible wrote: > 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. >

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

2019-05-08 Thread Bruno Haible
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: > >

Coverity false positives triggered by gnulib's implementation of base64

2019-05-07 Thread Kamil Dudka
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: