security review and patches for libpurple

[Ethan Blanton]

Ethan Blanton spake unto us the following wisdom:
> 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.

OK.  I apologize for the embarrassing delay on this, but I have just
landed thirteen EFF patches on im.pidgin.pidgin.  They are:

ZAsynclocate-strcpy.diff
ZInit-strcpy.diff
ZRetSubs-strcpy.diff
Zinternal-strcpy.diff
cipher-strcpy.diff
jabber-strcpy.diff
libc_interface-strcpy.diff
msn-strcpy.diff
posixuname-strcpy.diff
proxy-strcpy.diff
tcl_ref-strcpy.diff
tcl_signals-strcpy.diff
zephyr-strcpy.diff

Many of these have corrections in the form of more effective bounds
checks, fenceposts, and DRY memos.  I don't think there's anything
controversial here, but please have a look.  The bank of revisions is
(I believe) 28e56bcf through 269c6e29.

I have rejected the following patches for rejection, further review,
and/or correction:

* stringref_strcpy.diff

  This patch is just plain wrong.  It neglects to account for the one
  byte of the string being copied which resides in the stringref
  struct.  This is probably something the automated tool will forever
  be too stupid to understand.  The strlcpy() is entirely unnecessary
  here, because of the structure of the code -- and if it's messed up,
  other stuff is more messed up, so it's not fixing anything.

  Also, in general, this is NEVER safe:

    g_strlcpy(dest, src, strlen(src));

  That's even worse than strcpy(dest, src) with no check, because it
  checks *the wrong thing*.

* zephyr-protectoverflow.diff

  This may be OK, but I don't like it.  At the very least, it needs to
  reference G_MAXULONG or something of the sort.

* auth_cyrus.c

  This is bogus like the first patch; len is strlen(pw).  If we can
  determine the size of sasl_secret_t.data, we need to use that.  I
  haven't looked into this yet.

I have not yet processed the following patches at all:

filectl-sprintf.diff
imgstore-filename.diff
log-sscanf.diff
log-strcpy.diff
oscar-strcpy.diff
savedstatuses-strcpy.diff
stun-strcpy.diff
test_util-addtest.diff
util-strdup.diff
yahoo_filexfer-strcpy.diff
yahoo_packet-strcpy.diff

Sorry again for the delay.  I hope to get to those remaining patches
Soon, but I can't make any promises.  I'd like to see them in 2.10.0.

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/20110811/569146d5/attachment.pgp>