[GH-ISSUE #1887] Flawfinder static analysis

gitea-mirror commented

2026-05-05 07:45:39 -06:00

Owner

Originally created by @Vincent43 on GitHub (Apr 13, 2018).
Original GitHub issue: https://github.com/netblue30/firejail/issues/1887

I run Flawfinder static analysis tool for C/C++ on firejail sources. Here's the output: flawfinder.txt. I'm lacking of C expertise so I don't know if there is something interesting there, probably a lot of noise. I hope you find something useful.

EDIT: You can see more about what CWE-* means here.

Originally created by @Vincent43 on GitHub (Apr 13, 2018). Original GitHub issue: https://github.com/netblue30/firejail/issues/1887 I run [Flawfinder](https://www.dwheeler.com/flawfinder/) static analysis tool for C/C++ on firejail sources. Here's the output: [flawfinder.txt](https://github.com/netblue30/firejail/files/1907858/flawfinder.txt). I'm lacking of C expertise so I don't know if there is something interesting there, probably a lot of noise. I hope you find something useful. EDIT: You can see more about what `CWE-*` means [here](https://cwe.mitre.org/).

gitea-mirror added the

enhancement

label 2026-05-05 07:45:39 -06:00

gitea-mirror commented

2026-05-05 07:45:41 -06:00

Author

Owner

@SkewedZeppelin commented on GitHub (Apr 13, 2018):

👍
On the topic of similar tools we should look into them some more. I let afl run against firejail for a good two days a couple of months ago to no gain, however I'm pretty sure I didn't configure it very well.

@SkewedZeppelin commented on GitHub (Apr 13, 2018): :+1: On the topic of similar tools we should look into them some more. I let [afl](http://lcamtuf.coredump.cx/afl/) run against firejail for a good two days a couple of months ago to no gain, however I'm pretty sure I didn't configure it very well.

gitea-mirror commented

2026-05-05 07:45:44 -06:00

Author

Owner

@Fred-Barclay commented on GitHub (Apr 13, 2018):

On that note we can possibly integrate flawfinder, at least, with our Travis CI using the docker image: https://github.com/fkromer/docker-flawfinder

@Fred-Barclay commented on GitHub (Apr 13, 2018): On that note we can possibly integrate flawfinder, at least, with our Travis CI using the docker image: https://github.com/fkromer/docker-flawfinder

gitea-mirror commented

2026-05-05 07:45:45 -06:00

Author

Owner

@Vincent43 commented on GitHub (Apr 13, 2018):

In case of Travis CI integration we can silence false positives with /* Flawfinder: ignore */ comments i.e. strcpy(tmp, pScreenSize); /* Flawfinder: ignore */

@Vincent43 commented on GitHub (Apr 13, 2018): In case of Travis CI integration we can silence false positives with `/* Flawfinder: ignore */` comments i.e. `strcpy(tmp, pScreenSize); /* Flawfinder: ignore */`

gitea-mirror commented

2026-05-05 07:45:46 -06:00

Author

Owner

@smitsohu commented on GitHub (Apr 13, 2018):

I'm not sure if silencing a large number of warnings is the right approach.... if Travis CI integration is really desired, we should get rid of the warnings first. That would be a bigger effort, but doesn't mean it is not doable. It is only too much for one developer I guess... we would need a number of volunteers 😄

Another question is if it is worth it. The vast majority of these warnings are certainly false positives, in the sense that there is no corresponding bug.

@smitsohu commented on GitHub (Apr 13, 2018): I'm not sure if silencing a large number of warnings is the right approach.... if Travis CI integration is really desired, we should get rid of the warnings first. That would be a bigger effort, but doesn't mean it is not doable. It is only too much for one developer I guess... we would need a number of volunteers :smile: Another question is if it is worth it. The vast majority of these warnings are certainly false positives, in the sense that there is no corresponding bug.

gitea-mirror commented

2026-05-05 07:45:47 -06:00

Author

Owner

@Fred-Barclay commented on GitHub (Apr 14, 2018):

I don't mind working on changing the code if someone is willing to review all 314 159 pull requests I'll probably end up generating. 😆 I'm a python guy; I haven't caught the hang of C just yet and I need supervision!

Probably a lot are false positives but I'm not opposed to getting rid of them (if possible and within reason) anyways. :) Especially with firejail being setuid, we should probably take extra care and make sure our code is as tight as possible. I'm probably showcasing my C ignorance, but messsages like Does not check for buffer overflows (CWE-120) sound like something we should look into and change.

EDIT: also according to https://stackoverflow.com/a/1892556 some things we do (like using chmod instead of fchmod) present some risk since firejail has root permissions. It may not be easily exploitable for firejail (I have no idea), but it's worth handling IMHO.

@Fred-Barclay commented on GitHub (Apr 14, 2018): I don't mind working on changing the code if someone is willing to review all 314 159 pull requests I'll probably end up generating. :laughing: I'm a python guy; I haven't caught the hang of C just yet and I need supervision! Probably a lot are false positives but I'm not opposed to getting rid of them (if possible and within reason) anyways. :) Especially with firejail being setuid, we should probably take extra care and make sure our code is as tight as possible. I'm probably showcasing my C ignorance, but messsages like `Does not check for buffer overflows (CWE-120)` sound like something we should look into and change. EDIT: also according to https://stackoverflow.com/a/1892556 some things we do (like using chmod instead of fchmod) present some risk since firejail has root permissions. It may not be easily exploitable for firejail (I have no idea), but it's worth handling IMHO.

gitea-mirror commented

2026-05-05 07:45:49 -06:00

Author

Owner

@smitsohu commented on GitHub (Apr 14, 2018):

If I'm allowed a wish we should indeed look a bit closer at the races, though again I would expect most of them to be non-issues (e.g. functions dealing only with root stuff). At potentially overflowing functions I wouldn't look first.... even if we got a bug somewhere, chances are that the compiler takes care of it already with FORTIFY_SOURCE=2

@smitsohu commented on GitHub (Apr 14, 2018): If I'm allowed a wish we should indeed look a bit closer at the races, though again I would expect most of them to be non-issues (e.g. functions dealing only with root stuff). At potentially overflowing functions I wouldn't look first.... even if we got a bug somewhere, chances are that the compiler takes care of it already with [FORTIFY_SOURCE=2](https://access.redhat.com/blogs/766093/posts/1976213)

gitea-mirror commented

2026-05-05 07:45:51 -06:00

Author

Owner

@smitsohu commented on GitHub (Apr 14, 2018):

(if possible and within reason)

+1 from me. Flawfinder is obviously not especially smart as a tool, and following it without thinking doesn't make Firejail better necessarily.

Also if it turns out we are silencing hundreds of warnings because we consider them unfounded (and this is the most likely outcome), showcasing clean results then doesn't feel right. In fact I'm still not convinced of the Travis CI integration bit.

@smitsohu commented on GitHub (Apr 14, 2018): > (if possible and within reason) +1 from me. Flawfinder is obviously not especially smart as a tool, and following it without thinking doesn't make Firejail better necessarily. Also if it turns out we are silencing hundreds of warnings because we consider them unfounded (and this is the most likely outcome), showcasing clean results then doesn't feel right. In fact I'm still not convinced of the Travis CI integration bit.

gitea-mirror commented

2026-05-05 07:45:52 -06:00

Author

Owner

@Vincent43 commented on GitHub (Apr 14, 2018):

Here's slightly improved output (it shows offending line of code) - flawfinder -cQ firejail:
flawfinder-firejail.txt

@Vincent43 commented on GitHub (Apr 14, 2018): Here's slightly improved output (it shows offending line of code) - `flawfinder -cQ firejail`: [flawfinder-firejail.txt](https://github.com/netblue30/firejail/files/1910279/flawfinder-firejail.txt)

gitea-mirror commented

2026-05-05 07:45:53 -06:00

Author

Owner

@Fred-Barclay commented on GitHub (Apr 14, 2018):

Also if it turns out we are silencing hundreds of warnings because we consider them unfounded (and this is the most likely outcome), showcasing clean results then doesn't feel right. In fact I'm still not convinced of the Travis CI integration bit.

Yeah, that's just an idea for later (if at all). 😄

@Fred-Barclay commented on GitHub (Apr 14, 2018): > Also if it turns out we are silencing hundreds of warnings because we consider them unfounded (and this is the most likely outcome), showcasing clean results then doesn't feel right. In fact I'm still not convinced of the Travis CI integration bit. Yeah, that's just an idea for later (if at all). :smile:

gitea-mirror commented

2026-05-05 07:45:54 -06:00

Author

Owner

@smitsohu commented on GitHub (Apr 15, 2018):

I've spend some time with the provided list, I guess my comments above are not especially helpful. Please disregard them to the extent possible. I will continue looking at it in silence. Sorry for the distraction.

@smitsohu commented on GitHub (Apr 15, 2018): I've spend some time with the provided list, I guess my comments above are not especially helpful. Please disregard them to the extent possible. I will continue looking at it in silence. Sorry for the distraction.

gitea-mirror commented

2026-05-05 07:45:55 -06:00

Author

Owner

@matu3ba commented on GitHub (Apr 15, 2018):

grep -o "\(CWE-[0-9]*\)" flawfinder-firejail.txt | sort -n -k1.5 | uniq > CWE-list.txt
returns the according CWEs as list.
CWE-list.txt
Does flawfinder support different formatted output (CWE as first index)?
We could use the CWE as commits etc. Ideas?

@matu3ba commented on GitHub (Apr 15, 2018): `grep -o "\(CWE-[0-9]*\)" flawfinder-firejail.txt | sort -n -k1.5 | uniq > CWE-list.txt` returns the according CWEs as list. [CWE-list.txt](https://github.com/netblue30/firejail/files/1913488/CWE-list.txt) Does flawfinder support different formatted output (CWE as first index)? We could use the CWE as commits etc. Ideas?

gitea-mirror commented

2026-05-05 07:45:56 -06:00

Author

Owner

@SkewedZeppelin commented on GitHub (Apr 16, 2018):

@matu3ba I hope it doesn't, because I just made a script to reformat it and get the CWEs and their descriptions only.

Some seem to get a risk associated with them resulting in duplicates.

flawfinder-firejail-cwe_list.txt

@SkewedZeppelin commented on GitHub (Apr 16, 2018): @matu3ba I hope it doesn't, because I just made a script to reformat it and get the CWEs and their descriptions only. Some seem to get a risk associated with them resulting in duplicates. [flawfinder-firejail-cwe_list.txt](https://github.com/netblue30/firejail/files/1913525/flawfinder-firejail-cwe_list.txt)

gitea-mirror commented

2026-05-05 07:45:56 -06:00

Author

Owner

@matu3ba commented on GitHub (Apr 16, 2018):

@SkewedZeppelin Neat, since they are not alphanumerically ordered, heres the sorted form
grep "\(CWE-[0-9]*\)" flawfinder-firejail-cwe_list.txt | sort -n -k1.5 > flawfinder-firejail-cwe_list2.txt
There is still CWE-362: line twice which even uniq does not cut away.
flawfinder-firejail-cwe_list2.txt

@matu3ba commented on GitHub (Apr 16, 2018): @SkewedZeppelin Neat, since they are not alphanumerically ordered, heres the sorted form `grep "\(CWE-[0-9]*\)" flawfinder-firejail-cwe_list.txt | sort -n -k1.5 > flawfinder-firejail-cwe_list2.txt` There is still CWE-362: line twice which even `uniq` does not cut away. [flawfinder-firejail-cwe_list2.txt](https://github.com/netblue30/firejail/files/1914162/flawfinder-firejail-cwe_list2.txt)

Rows
Columns

[GH-ISSUE #1887] Flawfinder static analysis #1272