From owner-freebsd-security@FreeBSD.ORG Thu Apr 24 10:58:32 2014 Return-Path: Delivered-To: freebsd-security@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 975D5258 for ; Thu, 24 Apr 2014 10:58:32 +0000 (UTC) Received: from mail-qa0-x22d.google.com (mail-qa0-x22d.google.com [IPv6:2607:f8b0:400d:c00::22d]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 55F7B1899 for ; Thu, 24 Apr 2014 10:58:32 +0000 (UTC) Received: by mail-qa0-f45.google.com with SMTP id cm18so2050827qab.18 for ; Thu, 24 Apr 2014 03:58:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type:content-transfer-encoding; bh=RCKxP/eIvtxgUHAeGumCpuEYqIKo04GdJEQoyFTPI0Y=; b=biTutdC9mzbL18mq6CSt1ICQQi48OrlSzjsK832o7n3hHcmgMz9K9JKTNU7VVBsQ7b Cdnus/AeRiUB3qjeTA6+ZqocgA1J6QfqPtZrr/iWWeJH90Fd9rJWtniAPSQqXjdnJzx9 Zewjv/ICzhu/8G2DJJu6IDzl0QqiN0gwdBPEE3EOp9YOKj57z7Y38MtElKsP5H2UkK7F lSPt14cIwGuUTYcFKwYAOSNGKORzQ1ISzniQO+HhdVPKWdk8zYIuOIVw7s42X9OKA3sx cIStZVSvAYL4H3ibiQeW5I3QV5mp8RdR0cUKWWql98EZ54zVp/TPbznkEnw7q/hDULvi p6+A== MIME-Version: 1.0 X-Received: by 10.224.2.193 with SMTP id 1mr1390709qak.100.1398337111443; Thu, 24 Apr 2014 03:58:31 -0700 (PDT) Sender: benlaurie@gmail.com Received: by 10.96.162.196 with HTTP; Thu, 24 Apr 2014 03:58:31 -0700 (PDT) In-Reply-To: References: <10999.1398215531@server1.tristatelogic.com> <50CA7E78-BB5E-4872-A272-B7374627EC12@cederstrand.dk> Date: Thu, 24 Apr 2014 11:58:31 +0100 X-Google-Sender-Auth: mK7ZgJwbqSbLJnZcn_W8qwUO9mI Message-ID: Subject: Re: OpenSSL static analysis, was: De Raadt + FBSD + OpenSSH + hole? From: Ben Laurie To: Charles Swiger Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: "freebsd-security@freebsd.org security" , Erik Cederstrand X-BeenThere: freebsd-security@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "Security issues \[members-only posting\]" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Apr 2014 10:58:32 -0000 On 23 April 2014 20:14, Charles Swiger wrote: > Hi-- > > On Apr 23, 2014, at 3:06 AM, Erik Cederstrand = wrote: >> Den 23/04/2014 kl. 03.12 skrev Ronald F. Guilmette : > [ ... ] >>> 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"