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