Veracode static analysis results

[Elliott Sales de Andrade]

On Wed, Dec 5, 2012 at 4:22 PM, Ethan Blanton <elb at pidgin.im> wrote:
> Chris Wysopal spake unto us the following wisdom:
>> A customer asked us to analyze Pidgin using our static analyzer.  Our
>> responsible disclosure policy is to inform you of any findings so that
>> you may have the chance to review, comment, and/or fix the issues.
>>

It would probably be better if they checked the prpls too. Those're
more likely to have problems.

>> I think the software performed very well on our analysis but there are
>> a few issues we have found.  Attached is our full report. You can find
>> the description of the issues found on pages 10-15. We found 1 Very
>> High criticality. 5 Medium, and 47 low. Here is a summary.
>
> OK, here's my fifteen minute analysis of the bugs.  There's only one I
> think I'd really worry about.  I've not Cc'd veracode, we can send
> them our final conclusions.
>
> Very High:
>
> * gtkpounce error is a false positive.  Yes, we execute a user path
>   without verifying it, but that's the whole *point* of that feature.
>   It's not particularly safe, but only in an "enough rope to hang
>   yourself with" kind of way.
>
> Medium:
>
> * 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.
>
> * purple_core_migrate user-specified path is a false positive.  Pidgin
>   can already be coerced to read any file the user can read, and in the
>   general sense *should* be able to do so.  This class of bug simply
>   doesn't apply to Pidgin.
>
> * PurpleDesktopItem creation from file -- I don't even know what this
>   is.
>

I think this is only used in Pidgin for reading .desktop files that
are dropped on the conversation. If it's a link, then that link gets
added to the entry. Anything else seems to be ignored.

> * write_data_to_file path problem -- see purple_core_migrate
>
> * write_data_to_file race -- this is real.  We should be using open()
>   and fdopen() (or the g_ equivalents thereof?).
>
> Low:
>
> I'm not even going through these right now.  Some of them probably merit
> checks; the majority of the 47 are in imported code from glib or
> something, though.

Only 6 of them are in our code. I'm not sure what version of the code
they verified, but they don't match hg (3.0.0) and I didn't bother
looking at 2.10.7.

>
> Ethan
>

-- 
Elliott aka QuLogic
Pidgin developer