Date: Thu, 24 Apr 2014 11:58:31 +0100 From: Ben Laurie <benl@freebsd.org> To: Charles Swiger <cswiger@mac.com> Cc: "freebsd-security@freebsd.org security" <freebsd-security@freebsd.org>, Erik Cederstrand <erik+lists@cederstrand.dk> Subject: Re: OpenSSL static analysis, was: De Raadt + FBSD + OpenSSH + hole? Message-ID: <CAG5KPzxeupwCTK7-7oA1nhM7Q=Ggv-QCwBrNchM1wM3Hwvtv_w@mail.gmail.com> In-Reply-To: <B4A7F879-588B-4414-B416-601066C4E61A@mac.com> References: <10999.1398215531@server1.tristatelogic.com> <50CA7E78-BB5E-4872-A272-B7374627EC12@cederstrand.dk> <B4A7F879-588B-4414-B416-601066C4E61A@mac.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 23 April 2014 20:14, Charles Swiger <cswiger@mac.com> wrote: > Hi-- > > On Apr 23, 2014, at 3:06 AM, Erik Cederstrand <erik+lists@cederstrand.dk>= wrote: >> Den 23/04/2014 kl. 03.12 skrev Ronald F. Guilmette <rfg@tristatelogic.co= m>: > [ ... ] >>> I do imagine that the truth or falsehood of your assertion may depend >>> quite substantally on what one does or does not consider a "false >>> positive" in this context. >> >> Have a look at the ~10.000 reports at http://scan.freebsd.your.org/freeb= sd-head/ (unavailable ATM). Silly things are reported like missing return a= t the end of main() or not free()ing memory two lines before program exit. = There are nonsensical reports because the analyzer doesn't detect exit() in= a usage() function because usage() is defined in a separate compilation un= it, or this: > > Sure, static analysis isn't perfect and runs into false positives, some o= f which are truly harmless and some of which actually do indicate an area w= here refactoring the code in light of the warning would be an improvement. > > It's worth noting that even if you believe that (e.g.) the clang static a= nalyzer isn't properly doing liveness analysis and misjudging whether there= 's a dead assignment (writing to a variable which is never read), the clang= compiler will be using the same analysis when doing dead-code elimination = and common-subexpression elimination and such while optimizing. I think this is not true. I could be wrong, but I've actually worked on clang static analysis and I think it is an entirely separate system. Certainly there's no guarantee that a static analysis result will be reflected in the output of the compiler. >> int foo(int y, int z) { >> int x; >> if (y =3D=3D z) { >> x =3D 0; >> } else { >> if (y !=3D z) { >> x =3D 1; >> } >> } >> return x; >> } >> >> warning that x may be uninitialized. Fixing these require considerable e= ffort e.g. improving IPA and adding alpha-remaning support to the analyzer'= s constraint manager, or would result in unnecessary code churn in FreeBSD = just to work around the reports. > > Ah, that's a classic example. If you declared y and z as const, then I'd= agree that the compiler should be free to make assumptions that one of the= two if statements must be true. > > On the other hand, if you assume that the arguments are volatile and that= maybe another thread might update y or z on the stack between the time whe= n the first if test is evaluated and the second if, one realizes that the s= tatic analyzer might actually have a point. (Or you're on an SMP system an= d don't get sequential consistency / total-store ordering without memory ba= rriers....) > > Sure, your code might never intentionally try to mess with the stack, but= there's a long history of bugs like typing ~1030 characters at a password = prompt and blowing past a char passwd[1024] buffer that someone assumed wou= ld be more than enough. > > The most straightforward changes to this snippet would be either: > > int foo(int y, int z) { > int x; > if (y =3D=3D z) { > x =3D 0; > } else { > x =3D 1; > } > return x; > } > > ...or: > > int foo(int y, int z) { > int x =3D 0; > if (y !=3D z) { > x =3D 1; > } > return x; > } > > Not only are both of these shorter and they pass clang's static analyzer = without a warning, I'd argue that the second version is noticeably cleaner. > > Regards, > -- > -Chuck > > _______________________________________________ > freebsd-security@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-security > To unsubscribe, send any mail to "freebsd-security-unsubscribe@freebsd.or= g"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG5KPzxeupwCTK7-7oA1nhM7Q=Ggv-QCwBrNchM1wM3Hwvtv_w>