security review and patches for libpurple

[Mark Doliner]

Ethan I looked through your commits and they look good to me.

Comments about the remaining patches:
** imgstore-filename.diff **
This sanitizes filenames to be alphanumeric.  It seems like this would
make the filename much less useful to non-English speakers.  I'm not
convinced this is a good solution to this problem.  If we really think
this is a good idea then we should at least change the regexp to not
penalize international users so heavily.

** test_util-addtest.diff **
This adds a unit test.  I haven't seen anyone mention this one
recently.  It looks ok to me so I committed it.

** util-strdup.diff **
This adds a size check before allocating memory.  I'm don't like this
change.  If we're concerned about g_malloc() crashing if we try to
allocate too much memory then we should use g_try_malloc().  If we
want to avoid allocating a massive amount of memory then maybe we
should use setrlimit() to set the max amount of heap that our process
can use (possibly in conjunction with g_try_malloc()).

Then there's the question of what do you set the limit to (and your
suggested change has this same problem).  No matter what size you
choose you're setting an artificial limit for yourself, which always
has its pros and cons.  And setting a limit for a single allocation
doesn't buy you much--what if someone does something that causes you
to make hundreds of allocations that are each just smaller than the
limit?

We strdup and malloc memory all over the place.  Would you argue that
we should do this kind of check every time we allocate memory?  I
think this would quickly become cumbersome and error-prone.

This check is attempting to anticipate some sort of future remote
Denial of Service vulnerability, but I don't think it's worth it.  All
things considered I think this type of change makes users worse off.
The code becomes more complex and error prone, which potentially
INCREASES the chance of remotely-exploitable bugs.

If we're aware of a place where this function is used and there is a
potential to allocate a large chunk of memory, then we should handle
that in a more targeted change.

** yahoo_packet-strcpy.diff **
The code that constructs outgoing packets currently makes two passes
through the outgoing data--one to determine the buffer length needed,
and a second to write data to the buffer.  If we wanted to use
g_strlcpy() here then we would need to start passing around the buffer
end offset or maybe the buffer, current buffer position and buffer
length.  Maybe using a more advanced data structure.

** zephyr-protectoverflow.diff **
This adds a size check before allocating memory.  This is similar to
the change to util.c, but this one actually IS targeted, and so it
might be a good idea--I haven't looked at it in detail.  (Calling
exit() when read fails is definitely not acceptable.)

--Mark