Any static analysis tool, such as Flawfinder, is merely a tool. No tool can substitute for human thought! In short, "a fool with a tool is still a fool". It’s a mistake to think that analysis tools (like flawfinder) are a substitute for security training and knowledge. Developers – please read documents like my Secure Programming book so you’ll understand the vulnerabilities that the tool is trying to find! Organizations – please make sure your developers understand how to develop secure software (including learning about the common mistakes past developers have made), before having them develop software or use static analysis tools.
An example of horrific tool misuse is disabling vulnerability reports without (1) fixing the vulnerability, or (2) ensuring that it is not a vulnerability. It’s publicly known that RealNetworks did this with flawfinder; I suspect others have misused tools this way. I don’t mean to beat on RealNetworks particularly, but it’s important to apply lessons learned from others, and unlike many projects, the details of their vulnerable source code are publicly available. As noted in iDEFENSE Security Advisory 03.01.05 on RealNetworks RealPlayer (CVE-2005-0455), a security vulnerability was in this pair of lines:char tmp; /* Flawfinder: ignore */ strcpy(tmp, pScreenSize); /* Flawfinder: ignore */
This means that flawfinder did find this vulnerability, but instead fixing it, someone added the "ignore" directive to the code so that flawfinder would stop reporting the vulnerability. But an "ignore" directive simply stops flawfinder from reporting the vulnerability – it doesn’t fix the vulnerability! The intended use of this directive is to add it once a reviewer determined that it was definitely a false positive, but in this case the tool was reporting a real vulnerability. The same thing happened again in iDefense Security Advisory 06.23.05, where the vulnerable line was:sprintf(pTmp, /* Flawfinder: ignore */
And a third vulnerability with the same issue was reported still later in iDefense Security Advisory 06.26.07, RealNetworks RealPlayer/HelixPlayer SMIL wallclock Stack Overflow Vulnerability, where the vulnerable line was:strncpy(buf, pos, len); /* Flawfinder: ignore */
This is not to say that RealNetworks is a fool or set of fools. Indeed, I believe many organizations, not just RealNetworks, have misused tools this way. My thanks to RealNetworks publicly admitting their mistake – it allows others to learn from their mistake! My specific point is that you can’t just add comments with "ignore" directives and expect that the software is suddenly more secure. Do not add "ignore" directives until you are certain that the report is a false positive.
This kind of problem can easily happen in organizations that say "run scanning tools until there are no more warnings" but don’t later review the changes that were made to eliminate the warnings. If warnings are eliminated because code is changed to eliminate vulnerabilities, that’s great! General-purpose tools scanning like flawfinder will have false positive reports, though; it’s easy to create a tool without false positives, but they’ll do that by failing to report many possible vulnerabilities (some of which will really be vulnerabilities). The obvious answer if you want a broader tool is to allow developers to examine the code, and if they can truly justify that it’s a false positive, document why it is a false positive (say in a comment near the report) and then add a "Flawfinder: ignore" directive. But you need to really justify that the report is a false positive; just adding an "ignore" directive doesn’t fix anything! Sometimes it’s easier to fix a problem that may or may not be a vulnerability, instead of ensuring that it’s a false positive – the OpenBSD developers have been doing this successfully for years, since if complicated code isn’t an exploitable vulnerability yet, a tiny change can often turn such fragile code into a vulnerability.
If you’re in an organization using a scanning tool like this, make sure you review every change caused by a vulnerability report. Every change should be either (1) truly fixed or (2) correctly and completely justified as a false positive. I think organizations should require any such justification to be in comments next to the "ignore" directive. If the justification isn’t complete, don’t mark it with an "ignore" directive. And before developers even start writing code, get them trained on how to write secure code and what the common mistakes are; this material is not typically covered in university classes or even on the job.
最近大力鼓吹推行Code Review、Unit Test，其实经常看我博客的朋友都知道，我一直在讲这句话，提升代码质量的两个关键方法就是Code Review加上单元测试，也在自己的项目中身体力行，不过由于我只是小兵一个，所以只能自己做，还打不到让其他人也跟着做的程度。对于这两个工具，没有好的执行力推动，其实是不可能达到提升软件质量的效果的。用工具，用技术，但是最关键的一点是要用对工具，而且要有持之以恒的执行。
最简单的也最容易的就是从手头的项目开始，如果编译工具可以调整Warning，调到最高，然后fix掉这些Warning。如果遇到Bug，不要Ignore，先总结Root Cause，然后写Test case，改代码，编译，跑单元测试。只要持之以恒，不需要什么CEO总经理推动，产品质量自然就提升上来了。
《“A Fool with a Tool is still a Fool”》 有 1 条评论