security review and patches for libpurple

Paul Aurich paul at darkrain42.org
Fri Aug 12 00:25:59 EDT 2011


And Ethan Blanton spoke on 08/11/2011 08:19 AM, saying:
> * 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.

   * Intermingling g_* and libc allocation functions is a no-no (it breaks
on Win32, where glib doesn't use the underlying allocator -- annoying, but
such is life)
   * Exiting when a read fails seems icky.  Then again, this is Zephyr and
it's right after a select()

I'm assuming the special value (2^27) comes from being "big enough that
nobody would reasonably expect to allocate/read that much memory".  Is that
correct?  I'm a bit uneasy about this (and assuming that's what Ethan was
uneasy about), but I also think it's accurate that nobody would try to
legitimately need that big of a buffer in either of these functions.
Famous last words.

> * util-strdup.diff
>
>   This has the same problem as the Zephyr overflow protection patch.

If the string is large enough, it seems to me like 'i' would wrap around to
0 (I'd need to reason a bit more about this to convince myself the app
wouldn't die first) and spin in a loop.

I'm also trying to think of a way to avoid the extra strlen() -- does
checking if destsize < i work (destsize starts at 1)?

~Paul

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 900 bytes
Desc: OpenPGP digital signature
URL: <http://pidgin.im/cgi-bin/mailman/private/security/attachments/20110811/1b4bb5e3/attachment-0001.pgp>


More information about the security mailing list