Veracode static analysis results

[Mark Doliner]

Hi Chris!  Thanks again for performing this analysis and sending us
the result.  I've listed each flaw below along with our response.  One
other thing worth noting... it looks like you only analyzed
libpurple.so, libpurple-client.so and pidgin.  This omits all our code
for interacting with the various IM networks.  This code is compiled
into dynamically loadable modules named libmsn.so, libxmpp.so, etc.
You may wish to analyze these files, as well.


----- Very High, , Process Control (CWE ID 114),  Flaw Id 9043,
pidgin/gtkpounce.c 1515

We believe this is not a problem.  The analysis is correct, we are
executing a command without verifying it, but that's intentional.  The
command comes from the user of the application and is run as
themselves.  The user can do the same amount of harm as they could
with a terminal window.


----- Medium, Insufficient Entropy (CWE ID 331), Flaw Id 9027,
libpurple/ntlm.c 229

We changed this code to use /dev/urandom in Linux.  For now the code
continues to use rand() in Windows and anywhere else /dev/urandom
doesn't work.  The change is in our main development branch (what will
eventually become 3.0.0).  We're currently not planning on changing
this in 2.x.y, but we could be talked into it.  We think this is an
improvement, but realize it isn't perfect.  We're not very familiar
with NTLM and not sure how critical this session key is... it's only
used when authenticating to an HTTP proxy using NTLM and when
connecting to some servers using the SIMPLE protocol (possibly only
when connecting to Microsoft Live Communication Server).


----- Medium, External Control of File Name or Path (CWE ID 73), Flaw
Id 9018, libpurple/core.c 657

We believe this is not a problem.  The path can only be specified by
the local user running Pidgin.  We believe it is not harmful for
Pidgin/libpurple to attempt to open files specified by the user.
Adequate filesystem permissions should be used to prevent the user
gaining access to private files.


----- Medium, External Control of File Name or Path (CWE ID 73), Flaw
Id 8994, libpurple/desktopitem.c 1156

We believe this is not a problem.  The function in question is called
when the user drag-and-drops a file on their conversation window.  The
path can only be specified by the local user running Pidgin.  We
believe it is not harmful for Pidgin/libpurple to attempt to open
files specified by the user.  Adequate filesystem permissions should
be used to prevent the user gaining access to private files.


----- Medium, External Control of File Name or Path (CWE ID 73), Flaw
Id 9023, libpurple/util.c 2691

This one is more involved than the above two, because this utility
function is used from a lot of places.  I spent some time tracing
through the calls to this function and I think I found a few that are
problematic.  I'll send followup emails for them within the next few
days.


----- Medium, Time-of-check Time-of-use (TOCTOU) Race Condition (CWE
ID 367), Flaw Id 8989, libpurple/util.c 2683

We believe this problem is not severe, but I did change the function
to use fchmod() using the fd from the file handle, and it does it
before anything is written to the file.  We think this isn't severe
because in most (all?) cases the file being written is in the
directory ~/.purple/, and this directory should not be world readable.
 The change is in our main development branch (what will eventually
become 3.0.0).  We're currently not planning on changing this in
2.x.y.


----- Low, Unchecked Error Condition (CWE ID 391), Flaw Id 9085,
pidgin/gtkprefs.c 729
----- Low, Unchecked Error Condition (CWE ID 391), Flaw Id 9056,
pidgin/gtkprefs.c 732
----- Low, Unchecked Error Condition (CWE ID 391), Flaw Id 9055,
pidgin/gtkprefs.c 773
----- Low, Unchecked Error Condition (CWE ID 391), Flaw Id 9058,
pidgin/gtkprefs.c 780

We believe these are not serious problems, but I did add add error
logging for three of these (the fourth one is a little goofy and will
fail under some normal circumstances) in our main development branch
(what will eventually become 3.0.0).  We're currently not planning on
changing this in 2.x.y.


----- Low, Unchecked Error Condition (CWE ID 391), Flaw Id 8992,
libpurple/log.c 838

We believe this is not a serious problem, but I did add a logging
statement for this in our main development branch (what will
eventually become 3.0.0).  We're currently not planning on changing
this in 2.x.y.


----- Low, Unchecked Error Condition (CWE ID 391), Flaw Id 8996,
libpurple/util.c 1537

We believe this is not a problem.  We don't need to check the return
value here because an earlier if() statement ensures that the desired
character exists in the string.


----- Low, Unchecked Error Condition (CWE ID 391), <41 remaining flaws
in this category>

These are not in our code--they're in gio which is a part of glib.
It's a pretty widely-used library and I suspect these flaws aren't
serious.  We're not planning on taking any further action on these
(but if you think they're important then I suggest notifying the glib
developers).