Discussion:
[Bug-wget] [PATCH] Fixes for issues found by Coverity static analysis
Tomas Hozza
2018-08-24 16:24:04 UTC
Permalink
Hi.

We scanned the latest version of wget (1.19.5) with Coverity static analyzer. It found some potentially important issues like RESOURCE LEAKS. I'm attaching my proposed fixes for these issues. Each commit includes the output from Coverity and the outcome of my analysis of the problem from sources.

Regards,
Tomas
--
Tomas Hozza
Associate Manager, Software Engineering - EMEA ENG Core Services

PGP: 1D9F3C2D
UTC+1 (CET)
Red Hat Inc. http://cz.redhat.com
Darshit Shah
2018-08-25 06:20:10 UTC
Permalink
Hi Tomas,

Thanks for running the scan and the patches you've made! I briefly glanced
through those and they seem fine. Of course, they will need to be slightly
modified to apply to the current git HEAD. I can do that in the coming days and
apply these patches.

I would like to ask you if there is a regular scan of Wget that you have set up
on Coverity. We used to run coverity scans regularly, but since the last year
or so, I haven't managed to get the coverity binaries to execute on my system.
So the scans stopped. If you have a scheduled run, I would like to be able to
see the results on Coverity so that we can keep fixing those issues.

P.S.: It seems like you haven't assigned your copyrights to the FSF for Wget.
Do you happen to know if your employer has assigned the copyrights on your
behalf? I couldn't find any mentions in the list I have locally. You will
shortly receive the assignment form in a separate email.
Post by Tomas Hozza
Hi.
We scanned the latest version of wget (1.19.5) with Coverity static analyzer. It found some potentially important issues like RESOURCE LEAKS. I'm attaching my proposed fixes for these issues. Each commit includes the output from Coverity and the outcome of my analysis of the problem from sources.
Regards,
Tomas
--
Tomas Hozza
Associate Manager, Software Engineering - EMEA ENG Core Services
PGP: 1D9F3C2D
UTC+1 (CET)
Red Hat Inc. http://cz.redhat.com
--
Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6
Tomas Hozza
2018-08-27 09:01:14 UTC
Permalink
Hi Darshit.
Post by Darshit Shah
Hi Tomas,
Thanks for running the scan and the patches you've made! I briefly glanced
through those and they seem fine. Of course, they will need to be slightly
modified to apply to the current git HEAD. I can do that in the coming days and
apply these patches.
These were based on the git HEAD at the time of sending. From what I checked just now, that should be still the case. I'm working on git://git.savannah.gnu.org/wget.git.
Post by Darshit Shah
I would like to ask you if there is a regular scan of Wget that you have set up
on Coverity. We used to run coverity scans regularly, but since the last year
or so, I haven't managed to get the coverity binaries to execute on my system.
So the scans stopped. If you have a scheduled run, I would like to be able to
see the results on Coverity so that we can keep fixing those issues.
This is Red Hat's internal instance of Coverity combined with other static analyzers. Nevertheless I can share the full results with you if needed. Please let me know if I should send it to mailing list or to you directly.
Post by Darshit Shah
P.S.: It seems like you haven't assigned your copyrights to the FSF for Wget.
Do you happen to know if your employer has assigned the copyrights on your
behalf? I couldn't find any mentions in the list I have locally. You will
shortly receive the assignment form in a separate email.
My knowledge is that Red Hat has agreement with FSF covering all its employees. Since I'm a Red Hat employee and I'm sending these changes as part of my job, I consider this to be implied. I have contributed to wget in the past with the same rationale.

Regards,
Tomas
Post by Darshit Shah
Post by Tomas Hozza
Hi.
We scanned the latest version of wget (1.19.5) with Coverity static analyzer. It found some potentially important issues like RESOURCE LEAKS. I'm attaching my proposed fixes for these issues. Each commit includes the output from Coverity and the outcome of my analysis of the problem from sources.
Regards,
Tomas
--
Tomas Hozza
Associate Manager, Software Engineering - EMEA ENG Core Services
PGP: 1D9F3C2D
UTC+1 (CET)
Red Hat Inc. http://cz.redhat.com
--
Tomas Hozza
Associate Manager, Software Engineering - EMEA ENG Core Services

PGP: 1D9F3C2D
UTC+1 (CET)
Red Hat Inc. http://cz.redhat.com
Tim Rühsen
2018-08-27 10:59:55 UTC
Permalink
Post by Tomas Hozza
Hi Darshit.
Post by Darshit Shah
Hi Tomas,
Thanks for running the scan and the patches you've made! I briefly glanced
through those and they seem fine. Of course, they will need to be slightly
modified to apply to the current git HEAD. I can do that in the coming days and
apply these patches.
These were based on the git HEAD at the time of sending. From what I checked just now, that should be still the case. I'm working on git://git.savannah.gnu.org/wget.git.
Post by Darshit Shah
I would like to ask you if there is a regular scan of Wget that you have set up
on Coverity. We used to run coverity scans regularly, but since the last year
or so, I haven't managed to get the coverity binaries to execute on my system.
So the scans stopped. If you have a scheduled run, I would like to be able to
see the results on Coverity so that we can keep fixing those issues.
This is Red Hat's internal instance of Coverity combined with other static analyzers. Nevertheless I can share the full results with you if needed. Please let me know if I should send it to mailing list or to you directly.
Also a big "thank you" from my side !

If you think there is no obvious security issue involved, just send to
the ML. Otherwise to Darshit and me please.
Post by Tomas Hozza
Post by Darshit Shah
P.S.: It seems like you haven't assigned your copyrights to the FSF for Wget.
Do you happen to know if your employer has assigned the copyrights on your
behalf? I couldn't find any mentions in the list I have locally. You will
shortly receive the assignment form in a separate email.
My knowledge is that Red Hat has agreement with FSF covering all its employees. Since I'm a Red Hat employee and I'm sending these changes as part of my job, I consider this to be implied. I have contributed to wget in the past with the same rationale.
Sorry, that was my fault/doubt (asked Darshit and then was offline on
the weekend). Just found the entry in the FSF list of contributors. My
first grep was -i for 'redhat' - it actually is written 'Red Hat'.

Regards, Tim

Loading...