#10 Possible buffer overflows and security issues

[Pedro F. Giffuni]

Hi;

Considering the great job finding bugs done by Coverity
on other open source projects I went looking for
similar tools and I found FlawFinder, a very easy to
use security checker.

I ran flawfinder on BRLCAD 7.6.6 and since the US army
should be expected to keep up the finest standards in
security, I thought you might be interested in the
report it produced.

[Pedro F. Giffuni]

Flawfinder report (gzipped)

[Sean Morrison]

Logged In: YES
user_id=785737

That is excellent, Pedro.. thanks for running the tool and providing the
report. It reads about as verbosely as the output after using our configure's
--enable-warnings flag...

That's a pretty hefty laundry list, but definitely one worth looking into and
fixing. I've also had it planned to run valgrind on all the tools at some point
as well. I'm sure it will uncover more than a few issues. Since the list you
provided has a considerable number of high-risk warnings related to security,
this will definitely be getting some attention. Thanks again.

[Pedro F. Giffuni]

Logged In: YES
user_id=678384

Actually that file is not as nice as the html report ;-).

Many of these can be false positives, the tool is somewhat
dumb in that it just looks for functions that are known to
have security limitations. Valgrind is excellent for
memory leaks and double free's but coverity can find yet
more types of problems. Depending on developer's time we
could ask brlcad added to scan.coverity.com.

enjoy!

[Sean Morrison]

Logged In: YES
user_id=785737

Having BRL-CAD added to scan.coverity.com sounds like an excellent idea...
I'm looking into it. Thanks for the link!

[Sean Morrison]

Logged In: YES
user_id=785737
Originator: NO

FYI, as a follow up -- BRL-CAD was approved by and and added to the Coverity Scan project as of several months ago. We're already a "Rung 1" project and have a mailing list up for discussing the report issue, but we don't yet have a complete scan. I've been working with David Maxwell (of Coverity) to get our scan updated/fixed (which is why an announcement hasn't yet been made). More details to follow, but if folks are interested in working on resolving the scan issues, I'd be happy to set folks up with an account to our project (Coverity results are through a web-based per-project interface). Contact me via e-mail if interested.

[Sean Morrison]

Logged In: YES
user_id=785737
Originator: NO

Flawfinder is now integrated into BRL-CAD's regression testing system so that issues are auto-detected and considered testing failures depending on the level of the issue detected. All of the level 5 issues are now taken care of as well as most of the existing level 4 issues it detected. The remaining few 4's should be taken care of shortly but it's a pretty good effort that should be worthy of closing out this suport request item. Thanks again for seeding the idea and getting things started!

Cheers!
Sean

[Pedro F. Giffuni]

Logged In: YES
user_id=678384
Originator: YES

Hi again;

While I am absolutely delighted that things went so well with the flawfinder report and the new coverity scan, I think adding flawfinder to the regressions went too far: it does produce too many false positives (even at level 5). I think running it in once every 20 years is just about enough ;) ... besides you have coverity scans now.

[Sean Morrison]

Logged In: YES
user_id=785737
Originator: NO

Fortunately, the regression tests can certainly be changed very easily if they actually do become problematic, but it's already done some good. It detected when another developer inadvertently re-added a few calls to functions that shouldn't have been used shortly after being enabled that wouldn't have readily been caught otherwise. Since we've taken care of all the issues at a given level, there's limited concern of false positives as the only issues that will come up will be new code added. Either way, as I said, the test can be turned off on a moment's notice if it ever does become a problem. Having it run merely makes detection easier and will allow issues to be reviewered/resolved more quickly.

[Pedro F. Giffuni]

Logged In: YES
user_id=678384
Originator: YES

OK.. I'll be watching eagerly for the next version then ;).