Veracode static analysis results

[Ethan Blanton]

Mark Doliner spake unto us the following wisdom:
> On Wed, Dec 5, 2012 at 1:22 PM, Ethan Blanton <elb at pidgin.im> wrote:
> > * NTLM session key -- I don't know enough about NTLM to say if this is a
> >   real problem or not.  Using a real RNG certainly wouldn't hurt.
> 
> I'm assuming g_rand_int() has similar problems as rand()?  How big of
> a problem do we think this is?  From what I'm reading maybe
> /dev/random isn't perfect, but it's decent.  I'm inclined to think
> this is minor and doesn't need to be fixed soon (if ever).  If we were
> generating long-term SSL certificates or something, sure.  But in this
> case it seems like we're generating a key for only a single session.

/dev/random is very good.  Neither rand() nor g_rand_int() uses
/dev/random; they use some sort of long polynomial.  It's not suitable
for any sort of key generation, session or otherwise.  It may be fine
for a nonce or the like, but that depends heavily on the protocol.  I
don't know enough about NTLM (or the associated concerns) to opine.

> In case we think we need to change this: It looks like gnutls does
> provide a random number generator in their API.  libnss might not (at
> least, I couldn't find one).  We could possibly evaluate whether
> gnutls is widespread enough for us to drop support for libnss.

I don't think that's probably necessary.

> > * write_data_to_file race -- this is real.  We should be using open()
> >   and fdopen() (or the g_ equivalents thereof?).
> 
> How important do people think this is?  It seems not important to me.
> Worst case I can think of is that there is a brief window where
> accounts.xml is group and/or world readable.  But this shouldn't
> normally be a problem because ~/.purple/ should not be group or world
> writable, right?  Unless there are times when we write sensitive
> information outside of purple_user_dir using
> purple_util_write_data_to_file_absolute(), but I didn't notice any.

I think it is moderately important.

> I propose committing this fix to head and not 2.x.y.  Any objections?

This is fine with me.

Ethan