security review and patches for libpurple

[Ethan Blanton]

Dan Auerbach spake unto us the following wisdom:
> >Maybe.  That depends a *lot* on context.  This is a real disadvantage
> >of static analysis tools -- the person reading the output has to take
> >a careful look at what the code actually does.  There are other errors
> >in the patches which I believe are the result of static analysis
> >limitations (such as a +1 added to a malloc where it doesn't belong).
> >I bring this up as a general "gotcha" to keep an eye on, not as
> >specific criticism.
>
> I agree that looking at the code carefully is essential to good
> application of static analysis tools. And though I tried to do this,
> clearly my brain was off some of the time :) But I'd still say that
> eliminating unsafe functions is a good idea, independent of the
> valid limitations of static analysis.

Completely agreed.  I hope I did not give any other impression.  :-)
This is good work, and valuable, and should absolutely be tackled.  We
just might have to tackle it slightly differently, in places, to keep
me happy.  ;-)

> >I have corrected these patterns in the patches where they occurred,
> >and I have corrected fencepost errors where they occurred.  I've
> >applied about half of the patches with or without some modification.
> >Some of the patches have more serious errors or make assumptions I
> >need to validate before they can be applied.  I hope to send a list of
> >those patches and my specific concerns "soon".
> 
> Thanks for looking at this in detail and applying the superior
> solution where possible. I should confess that the strlcpy part of
> the series of patches we sent is probably the most error-prone,
> relied most heavily on static analysis, and given the errors you've
> found, probably should be taken more as a guide.

There are only two (I think?) strlcpy patches that I'm going to reject
from this round, all of the others are applied (possibly with some
changes or another to make them more robust in context).  The ones
which I'm going to kick out for this round do hilight places we should
fix, I just think we should factor the problem out differently to
eliminate it.  I hope to have commentary on the patches I don't apply
soon.  In fact, I may document this stuff before I finish all of the
patches, just so they don't get lost in the shuffle.

With that in mind, I'd like to ask again if there are any objections
to my committing these patches to ipp without embargo or a coordinated
release.  If not, I will land them some time tomorrow.  If anyone even
simply thinks we should wait a few days or get additional input before
landing them, that's fine, too.

Ethan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 482 bytes
Desc: Digital signature
URL: <http://pidgin.im/cgi-bin/mailman/private/security/attachments/20110717/6817f281/attachment.pgp>