Adding Flawfinder to Patchwatcher

[Austin English]

I had a discussion with Dan about adding Flawfinder to the
patchwatcher. Currently, it's got some pretty generic errors, but it
seems able to test only patches, so we wouldn't be flooded with old
nonbugs (or we could set up a blacklist of safe errors). For
reference, I've run it on today's git. I'm attaching the full log, as
well as a condensed version of the most common errors (1 per error
type). Looks like a lot of chances for buffer overflows..

Thoughts?

-Austin

[Reece Dunn]

2008/8/28 Austin English :
> I had a discussion with Dan about adding Flawfinder to the
> patchwatcher. Currently, it's got some pretty generic errors, but it
> seems able to test only patches, so we wouldn't be flooded with old
> nonbugs (or we could set up a blacklist of safe errors). For
> reference, I've run it on today's git. I'm attaching the full log, as
> well as a condensed version of the most common errors (1 per error
> type). Looks like a lot of chances for buffer overflows..
>
> Thoughts?

+1

This looks good, but there does seem to be a large amount of noise and
it seems to generate warnings without being able to identify correct
usage.

These issues will need to be verified (i.e. the NULL DACLs used in the
tests and the potential buffer overflows).

It would be interesting to see what results sparse and smatch
generate, and if they (or valgrind) can be extended to identify these
(with the possibility of ignoring them on correct usage) and more.

This could also be extended to the resources: checking that there are
no duplicate mnemonics, that controls that need a label have one and
other usability issues.

- Reece

[Rob Shearman]

2008/8/28 Austin English :
> I had a discussion with Dan about adding Flawfinder to the
> patchwatcher. Currently, it's got some pretty generic errors, but it
> seems able to test only patches, so we wouldn't be flooded with old
> nonbugs (or we could set up a blacklist of safe errors). For
> reference, I've run it on today's git. I'm attaching the full log, as
> well as a condensed version of the most common errors (1 per error
> type). Looks like a lot of chances for buffer overflows..
>
> Thoughts?

Too many false positives to make it worth using. Just because you use
strcpy, for example, it doesn't mean your program has a chance for a
buffer overflow; it's using strcpy with a destination buffer that
might not be large enough that causes buffer overflows.

Having some kind of static analysis done on patches before Alexandre
commits them is a good idea, but I don't think Flawfinder is the right
static analysis tool to use.

Coverity and Prefast are both static analysis tools with a bit more
intelligence that identify bad code rather than just using "bad"
functions. Other people may be able to suggest more good tools.

[Kai Blin]

On Sunday 31 August 2008 15:03:38 Rob Shearman wrote:
> 2008/8/28 Austin English :
> > I had a discussion with Dan about adding Flawfinder to the
> > patchwatcher. Currently, it's got some pretty generic errors, but it
> > seems able to test only patches, so we wouldn't be flooded with old
> > nonbugs (or we could set up a blacklist of safe errors). For
> > reference, I've run it on today's git. I'm attaching the full log, as
> > well as a condensed version of the most common errors (1 per error
> > type). Looks like a lot of chances for buffer overflows..
> >
> > Thoughts?
>
> Too many false positives to make it worth using. Just because you use
> strcpy, for example, it doesn't mean your program has a chance for a
> buffer overflow; it's using strcpy with a destination buffer that
> might not be large enough that causes buffer overflows.

Ack. Just checking for the part of Wine that I know best:

wine-git/dlls/secur32/dispatcher.c:104:  [4] (shell) execvp: This causes a new 
program to execute and is difficult to use safely. try using a library call 
that implements the same functionality if available.

Arguably correct, but no way to fix it. This is expected noise.

wine-git/dlls/secur32/dispatcher.c:119:  [4] (crypto) crypt: Function crypt is 
a poor one-way hashing algorithm; since it only accepts passwords of 8 
characters or less, and only a two-byte salt, it is excessively vulnerable to 
dictionary attacks given today's faster computing equipment. Use a different 
algorithm, such as SHA-1, with a larger non-repeating salt.

This one is just rubbish. For your convenience, line 119 of dispatcher.c looks 
like this:
        helper->crypt.ntlm.a4i = NULL;

Now, given that there's a struct in the helper struct called "crypt" and 
flawfinder triggers on that, there's a ton of repeated useless warnings, as 
flawfinder doesn't even notice this isn't a function call. What does that 
tool do? grep over the sources for a blacklist of strings?

wine-git/dlls/secur32/wrapper.c:568:  [4] (access) ImpersonateSecurityContext: 
If this call fails, the program could fail to drop heightened privileges. 
Make sure the return value is checked, and do not continue if a failure is 
reported.

Er, duh? It's nice to see the blacklist includes win32 function calls as well. 
Just a bit pointless for the implementation of this function. We'll probably 
see this for other "potentially dangerous" functions as well. This would make 
sense if and only if this warning only would trigger if the return value 
wasn't checked, not on a plain string match.

So while I agree that intelligent checking of patches is a nice thing to have, 
I'm not convinced flawfinder is intelligent enough.

Cheers,
Kai

[Jason Spiro]

Rob Shearman <robertshearman <at> gmail.com> wrote:
> Coverity and Prefast are both static analysis tools with a bit more
> intelligence that identify bad code rather than just using "bad"
> functions.

I'm pretty sure Coverity's tool, Prevent, costs money to buy.  IIRC if the
maintainers of an OSS project ask them to, they will set their
http://scan.coverity.com server to run a scan on that project's code and upload
the scan results to the web at no charge.  But their tool will always remain
closed-source (unmodifiable) software.

Prefast is closed-source freeware for Windows.  IIRC they ship it as part of a
package called "Prefast for Drivers".  The package includes Prefast, plus a
Prefast plugin for scanning Windows hardware drivers.  You don't have to use the
plugin.
> Other people may be able to suggest more good tools.

AFAIK "splint" is one of the most popular OSS static analysis tools, but I've
never really used it.  Has anyone here used it?  On the flawfinder homepage, it
says that splint does deeper analysis than flawfinder.  It says it "...works
somewhat like lint, searching for probable errors; to really use it, developers
need to add additional annotations to help the tool identify problems. This is a
very mature program, widely used, and one you can start using right away on
'real programs'."

[Juan Lang]

> AFAIK "splint" is one of the most popular OSS static analysis tools, but I've
> never really used it.  Has anyone here used it?  On the flawfinder homepage, it
> says that splint does deeper analysis than flawfinder.  It says it "...works
> somewhat like lint, searching for probable errors; to really use it, developers
> need to add additional annotations to help the tool identify problems. This is a
> very mature program, widely used, and one you can start using right away on
> 'real programs'."

splint is merely one of the first, but hardly one of the most popular.
 It requires far too much code annotation to be useful.  There are
other more recent static analysis tools (cqual, or more recently oink,
come to mind) that could be used, but they're not that useful "out of
the box."  Rules for common things would need to be written before the
tools would be useful.

The analysis Coverity did was pretty high quality in comparison to
other tools I've tried.  The trouble with Coverity's scans was lack of
developer time to analyze the results, I think.  Even a relatively
good tool has false positives, and it takes time sort out the real
bugs from the noise.

Also, smatch is based on the same paper (the Stanford checker,
metacomp) that Coverity's tool started as.  At least Michael Stefaniuc
uses it to some success around here, and it could be extended to cover
more cases.
--Juan

splint (Re: Adding Flawfinder to Patchwatcher)

[Detlef Riekenberg]

On So, 2008-09-07 at 18:10 +0000, Jason Spiro wrote:
> > Other people may be able to suggest more good tools.
>
> AFAIK "splint" is one of the most popular OSS static analysis tools, but I've
> never really used it.  Has anyone here used it?  On the flawfinder homepage, it
> says that splint does deeper analysis than flawfinder.  It says it "...works
> somewhat like lint,

Use grep / read the source:
  dnl Check for lint
  AC_CHECK_PROGS(LINT, lclint lint)
...

But I think, thats unused for years...
(and it does not work for splint)

Repeated messages (was Re: Adding Flawfinder to Patchwatcher)

[Erik de Castro Lopo]

Austin English wrote:
> I had a discussion with Dan about adding Flawfinder to the
> patchwatcher.

Is anyone else getting multiple copies of this message? I seem
to be getting one every 10-20 minutes.

A quick scan of the email headers suggests that this is getting
resent from within the Gmail system. Each duplicate has a received
line like:

Received: by ey-out-1920.google.com with SMTP id 4so195432eyg.30
    for ; Thu, 28 Aug 2008 20:50:50 -0700 (PDT)

with a unique SMTP id.

Erik

Repeated messages (was Re: Adding Flawfinder to Patchwatcher)

[Austin English]

On Sun, Aug 31, 2008 at 6:38 PM, Erik de Castro Lopo
 wrote:
> Austin English wrote:
> > I had a discussion with Dan about adding Flawfinder to the
> > patchwatcher.
>
> Is anyone else getting multiple copies of this message? I seem
> to be getting one every 10-20 minutes.
>
> A quick scan of the email headers suggests that this is getting
> resent from within the Gmail system. Each duplicate has a received
> line like:
>
> Received: by ey-out-1920.google.com with SMTP id 4so195432eyg.30
>        for ; Thu, 28 Aug 2008 20:50:50 -0700 (PDT)
>
> with a unique SMTP id.
> Erik
> Erik de Castro Lopo
>
> Beware the Lollipop of Mediocrity.
> Lick it once, and you suck forever.

I received a few messages telling me it didn't go through, then after
a few days it did. I didn't resend it at all though...

-Austin

Repeated messages (was Re: Adding Flawfinder to Patchwatcher)

[Wesley Parish]

On Tuesday 02 September 2008 07:33, Austin English wrote:
> On Sun, Aug 31, 2008 at 6:38 PM, Erik de Castro Lopo
>
>  wrote:
> > Austin English wrote:
> > > I had a discussion with Dan about adding Flawfinder to the
> > > patchwatcher.
> >
> > Is anyone else getting multiple copies of this message? I seem
> > to be getting one every 10-20 minutes.
> >
> > A quick scan of the email headers suggests that this is getting
> > resent from within the Gmail system. Each duplicate has a received
> > line like:
> >
> > Received: by ey-out-1920.google.com with SMTP id 4so195432eyg.30
> >        for ; Thu, 28 Aug 2008 20:50:50 -0700 (PDT)
> >
> > with a unique SMTP id.
> > Erik
> > Erik de Castro Lopo
> >
> > Beware the Lollipop of Mediocrity.
> > Lick it once, and you suck forever.
>
> I received a few messages telling me it didn't go through, then after
> a few days it did. I didn't resend it at all though...
>
> -Austin

Which probably accounts for it arriving en masse.  And if that behaviour was 
repeated once or or more ...

Wesley Parish

Repeated messages (was Re: Adding Flawfinder to Patchwatcher)

[L. Rahyen]

On Sunday August 31 2008 23:38:00 Erik de Castro Lopo wrote:
> Austin English wrote:
> > I had a discussion with Dan about adding Flawfinder to the
> > patchwatcher.
>
> Is anyone else getting multiple copies of this message? I seem
> to be getting one every 10-20 minutes.

Yes, I can confirm this. Some time ago copies of this message 
stopped coming. However, this was annoying because the message 
contains "heavy" attachment. I hope this will not happen again...

Repeated messages (was Re: Adding Flawfinder to Patchwatcher)

[Chris Ahrendt]

Erik de Castro Lopo wrote:
> Austin English wrote:
> > I had a discussion with Dan about adding Flawfinder to the
> > patchwatcher.
>
> Is anyone else getting multiple copies of this message? I seem
> to be getting one every 10-20 minutes.
>
> A quick scan of the email headers suggests that this is getting
> resent from within the Gmail system. Each duplicate has a received
> line like:
>
> Received: by ey-out-1920.google.com with SMTP id 4so195432eyg.30
>     for ; Thu, 28 Aug 2008 20:50:50 -0700 (PDT)
>
> with a unique SMTP id.
>
> Erik

I am getting tonnes of them.. also I am getting refused connections from 
  the git repository.. I can ping the server but the connection is refused.

Chris

Repeated messages (was Re: Adding Flawfinder to Patchwatcher)

[Eliot Blennerhassett]

Erik de Castro Lopo wrote:
> Austin English wrote:
> > I had a discussion with Dan about adding Flawfinder to the
> > patchwatcher.
>
> Is anyone else getting multiple copies of this message? I seem
> to be getting one every 10-20 minutes.

Yep.  Overflowed my mailbox quota while I was away on vacation :(
And now proceeding to try it again... grr.

[Wesley Parish]

One thing you might do is check your email settings.  I received at least 
twenty coppies of this email - twenty * 3.1 MB.  Not nice.

Wesley Parish

On Friday 29 August 2008 08:59, Austin English wrote:
> I had a discussion with Dan about adding Flawfinder to the
> patchwatcher. Currently, it's got some pretty generic errors, but it
> seems able to test only patches, so we wouldn't be flooded with old
> nonbugs (or we could set up a blacklist of safe errors). For
> reference, I've run it on today's git. I'm attaching the full log, as
> well as a condensed version of the most common errors (1 per error
> type). Looks like a lot of chances for buffer overflows..
>
> Thoughts?
>
> -Austin

[Hans Petter Selasky]

Hi,

I've received +50 copies of this e-mail now with full attachments flooding my 
inbox. Is there a bug at my ISP or is there a bug in the mailman software ?

Thanks,
--HPS

[Austin English]

On Mon, Sep 1, 2008 at 12:03 PM, Hans Petter Selasky  wrote:
> Hi,
>
> I've received +50 copies of this e-mail now with full attachments flooding my
> inbox. Is there a bug at my ISP or is there a bug in the mailman software ?
>
> Thanks,
> --HPS

Someone else mentioned this as well. I didn't send multiple, and I
didn't receive any, though I have mailman prefs set to not send me a
copy when I'm author/cc.

If I need to notify Gmail, someone please let me know.

-Austin

[Francois Gouget]

On Mon, 1 Sep 2008, Austin English wrote:
[...]
> Someone else mentioned this as well. I didn't send multiple, and I
> didn't receive any, though I have mailman prefs set to not send me a
> copy when I'm author/cc.
>
> If I need to notify Gmail, someone please let me know.

You're not the only one whose emails got sent multiple times. There are 
a couple of other victims. I didn't check if Gmail was the common point.

[Francois Gouget]

On Tue, 2 Sep 2008, Francois Gouget wrote:
> On Mon, 1 Sep 2008, Austin English wrote:
> [...]
> > Someone else mentioned this as well. I didn't send multiple, and I
> > didn't receive any, though I have mailman prefs set to not send me a
> > copy when I'm author/cc.
> >
> > If I need to notify Gmail, someone please let me know.
>
> You're not the only one whose emails got sent multiple times. There are 
> a couple of other victims. I didn't check if Gmail was the common point.

I checked this time around and Gmail is the common denominator. But not 
all Gmail emails are impacted.

[Erik de Castro Lopo]

Hans Petter Selasky wrote:
> Hi,
>
> I've received +50 copies of this e-mail now with full attachments flooding my 
> inbox. Is there a bug at my ISP or is there a bug in the mailman software ?

Its not your ISP and its not the mailman software, its Gmail.

Erik

[Darragh Bailey]

On Mon, Sep 01, 2008 at 07:03:20PM +0200, Hans Petter Selasky wrote:
> Hi,
>
> I've received +50 copies of this e-mail now with full attachments flooding my 
> inbox. Is there a bug at my ISP or is there a bug in the mailman software ?
>
> Thanks,
> --HPS

Most likely a problem on the gmail side. All emails for the last day or
two from people using gmail accounts to send mail to this list has
resulted in duplicate emails.