Date: Tue, 22 Apr 2014 19:22:06 -0600 From: Chad Perrin <code@apotheon.net> To: "Ronald F. Guilmette" <rfg@tristatelogic.com>, freebsd-security@freebsd.org Subject: Re: OpenSSL static analysis, was: De Raadt + FBSD + OpenSSH + hole? Message-ID: <20140423012206.GB8271@glaze.hydra> In-Reply-To: <20140423010054.2891E143D098@rock.dv.isc.org> References: <DC2F9726-881B-4D42-879F-61377CA0210D@mac.com> <8783.1398202137@server1.tristatelogic.com> <20140423003400.GA8271@glaze.hydra> <20140423010054.2891E143D098@rock.dv.isc.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Apr 23, 2014 at 11:00:54AM +1000, Mark Andrews wrote: > In message <20140423003400.GA8271@glaze.hydra>, Chad Perrin writes: > > On Tue, Apr 22, 2014 at 02:28:57PM -0700, Ronald F. Guilmette wrote: > > > In message <DC2F9726-881B-4D42-879F-61377CA0210D@mac.com>, > > > Charles Swiger <cswiger@mac.com> wrote: > > > > > > > >Bug Type Quantity > > > >All Bugs 182 > > > > > > > >Dead store > > > > Dead assignment 121 > > > > Dead increment 12 > > > > Dead initialization 2 > > > > > > > >Logic error > > > > Assigned value is garbage or undefined 3 > > > > Branch condition evaluates to a garbage value 1 > > > > Dereference of null pointer 27 > > > > Division by zero 1 > > > > Result of operation is garbage or undefined 9 > > > > Uninitialized argument value 2 > > > > Unix API 4 > > > > > > Thank you for doing this. > > > > > > Perhaps it goes without aying, but I'll say it anyway. The above results > > > are at once both enlightening and disgusting. > > > > > > Apparently, the OpenBSD guys are reorganizing/rewriting OpenSSL. I hope > > > that they take the time to do what you have done *and* also to drive every > > > bleedin' last one of these numbers to zero. I feel sure that the vast > > > majority of the issues uncovered by clang are not in any sense exploitable, > > > however its the one or two or three that are that worry me. > > > > LibreSSL (re: reorganizing/forking OpenSSL). I'm sure they'll be doing > > a very thorough job, as LibreSSL will apparently be added to the full > > body of code managed and extensively code-reviewed by the OpenBSD > > project. The developers are also taking the encouraging first step of > > throwing away eight metric tons of cruft. I do not know whether they > > plan to specifically use Clang's static analyzer as an aid to their > > efforts, but I would guess they'll be taking similar measures. > > > > From what I have seen (which, admittedly, is pretty superficial by many > > standards), it looks like OpenSSL is probably just the best of a really > > bad breed of software. The most widespread, popular, by some standard > > "major" TLS packages all seem to be rancid crap, with OpenSSL just being > > the marginally least rancid of the lot. If something like the heartbeat > > leak exists in OpenSSL, I fully expect that the other big players have > > worse problems. Consider for instance that (real-world impact aside) > > the heartbeat bug was the result of a failure of secure coding that took > > the form of a fairly common way for people to overlook memory management > > problems in C, > > Heartbleed had NOTHING to do with memory managment. It was a input > parsing bug plain and simple. Standard malloc would not have helped > one iota. Even turning on no standard malloc features like overwrite > on free was not guarenteed to help. All that does is reduce the > amount of memory with data that could be read. It does not prevent > active memory being read. The first sentence of that paragraph depends for its accuracy on your definition of memory management. If you exclude "allowing improper access to memory by failing to check bounds on requests" from "memory management", then sure, it had "NOTHING" to do with memory management. I took a somewhat looser approach to defining "memory management" in my statement. I apologize for being the hapless stranger to set off your negative reaction to use of the term "memory management" for anything other than allocating and de-allocating memory. > > As for the number of CLANG analysis warnings. Clang has false > positives some of which are impossible to remove regardless of how > you recode the section and most of what was reported will be benign. I don't know why you're saying that in response to *me*. I didn't really say anything about the bug count in the Clang static analysis. > > We use clang, Coverity and other tools all the time on products we > ship. We use multiple compilers and each one of them pick up > different things. Even after doing all that and removing all the > warning we can we have to mark things as "Ingore - false positive" > or "Ignore - Intentional" as the analyser doesn't follow all of the > assignments or doesn't handle initialisation code that doesn't need > to be locked or ..... Obviously, human judgment is an important part of the process of finding and fixing bugs. If it wasn't, the last program we'd ever have to debug would be the one that finds and fixes bugs. . . . and? -- Chad Perrin [ original content licensed OWL: http://owl.apotheon.org ]
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140423012206.GB8271>