security review and patches for libpurple

[Ethan Blanton]

It's me again!

I have now applied these patches:

dnssrv-strcpy.diff
filectl-sprintf.diff
log-sscanf.diff
oscar-strcpy.diff
stun-strcpy.diff

I have rejected these patches:

* log_strcpy.diff

  This is just wrong.  It doesn't check anything which is actually
  safe, and we're replacing exactly three characters with exactly
  three characters, which is always safe anyway.

* savedstatuses-strcpy.diff

  Essentially the same problem as log_strcpy.diff, except that the
  replacement space is always >= the replacement string length, not
  exactly the same.

* yahoo_filexfer-strcpy.diff

  Not only is the original code here stupid, but the patch doesn't fix
  it.  ;-)  While the original code is stupid, it's not *wrong*, just
  inefficient and obtuse.  I fixed it to:

  time_str[strlen(time_str) - 1] = '\0';

* yahoo_packet-strcpy.diff

  This has the problem of several other patches, in that it bounds
  checks the copy based on the length of the source string, not
  destination buffer.  This is worse than what is already there.

* imgstore-filename.diff

  This needs work, or at least verification.  I'm not sure that the
  character set being allowed for the destination filenames is kosher.
  It may be, but someone needs to look at that.

* util-strdup.diff

  This has the same problem as the Zephyr overflow protection patch.

I think that's all of the patches.  As a summary, we applied:

ZAsynclocate-strcpy.diff
ZInit-strcpy.diff
ZRetSubs-strcpy.diff
Zinternal-strcpy.diff
cipher-strcpy.diff
dnssrv-strcpy.diff
filectl-sprintf.diff
jabber-strcpy.diff
libc_interface-strcpy.diff
log-sscanf.diff
msn-strcpy.diff
oscar-strcpy.diff
posixuname-strcpy.diff
proxy-strcpy.diff
stun-strcpy.diff
tcl_ref-strcpy.diff
tcl_signals-strcpy.diff
zephyr-strcpy.diff

These were just bogus:

bonjour-geteuid.diff
log-strcpy.diff
savedstatuses-strcpy.diff
stringref-strcpy.diff
yahoo_filexfer-strcpy.diff

We can ignore those patches from here on out, I think.  The following
patches may require some more attention.

These need work:

auth_cyrus-strcpy.diff
util-strdup.diff
yahoo_packet-strcpy.diff
zephyr-protectoverflow.diff

This one needs someone to verify that it's OK/correct:

imgstore-filename.diff

Any comments or suggestions are welcome.

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/50e3345f/attachment.pgp>