Veracode static analysis results
elb at pidgin.im
Sat Dec 29 20:35:58 EST 2012
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.
More information about the security