[Geany-Devel] CodeAi Fixes a Null Pointer Dereference

[Lex Trotman]

On 12 May 2017 at 05:40, Benjamin Bales <benjamin.bales at qbitlogic.com>
wrote:

> Lex and Matthew,
>
> You seem to disagree on this issue.  Is this a valid issue?  And if so, do
> you like the fix?  I can batch a few of them (<= 10) as a single PR.  No
> problem.
>

​Matthew is right, I misread the report as saying that `doc` was possibly
NULL, not `file_name`.  At the moment the project members are having a
crisis of availability, so at the moment smaller is better.  Checking a
single one quickly is more likely to fit into someones day.

Cheers
Lex​



>
> -Ben
>
> On Wed, May 10, 2017 at 8:22 PM, Matthew Brush <mbrush at codebrainz.ca>
> wrote:
>
>> On 2017-05-10 04:09 PM, Lex Trotman wrote:
>>
>>> On 11 May 2017 at 08:10, Benjamin Bales <benjamin.bales at qbitlogic.com>
>>> wrote:
>>>
>>> CodeAi (https://github.com/C0deAi), an automated repair tool developed
>>>> at
>>>> QbitLogic (www.qbitlogic.com), suggested the following fix. Could I
>>>> submit it as a patch if it looks alright?
>>>>
>>>> plugins/saveactions.c: “doc->file_type” pointer might be dereferenced
>>>> when
>>>> null on line 283.  Initialization may be provided by “doc” passed in as
>>>> a
>>>> function argument, but a null check would be prudent just in case. The
>>>> fix
>>>> checks “doc->file_type” for null before allowing a dereference on the
>>>> following line.  A snapshot of the bug report generated by CodeAi is
>>>> attached.  A full report is available upon request.
>>>>
>>>>
>>> This function is called (via the signal framework) by the function that
>>> created `doc` and as such cannot be null.  The design of the application
>>> uses the signal framework to decouple caller and callee and this is
>>> likely
>>> to confuse your tool since it cannot see where functions are called.
>>> Whilst any contributions are welcome, a report with a lot of similar
>>> false
>>> positives may end up being ignored and be a bad advertisement for your
>>> tool.
>>>
>>>
>> Naw, I think it's technically a real bug, albeit very minor. It's the
>> `file_type` member of the `doc` that can be NULL. IIUC tools like this look
>> to see if you checked the NULL-ness of something and then proceed to
>> dereference it outside of that check later, which this code does (checks if
>> `ft == NULL` several lines up and then unconditionally dereferences it on
>> the line given by the OP).
>>
>> Regards,
>> Matthew Brush
>>
>>
>> _______________________________________________
>> Devel mailing list
>> Devel at lists.geany.org
>> https://lists.geany.org/cgi-bin/mailman/listinfo/devel
>>
>
>
>
> --
> Benjamin Bales
> Chief Technology Officer
> [image: QbitLogic]
> 1050 Crown Pointe Pkwy, Ste. 840
> Atlanta, GA 30338
> 470-554-2690
>
> CONFIDENTIALITY NOTICE
>
> This e-mail and any files transmitted with it are confidential and are
> intended solely for the use of the individual or entity to which they are
> addressed.  This communication may contain privileged attorney material or
> other Property and Confidential matter.  If you are not the intended
> recipient or the person responsible for delivering the e-mail for the
> intended person, be advised that you have received this e-mail in error and
> that any use, dissemination, forwarding, printing, or copying of this
> e-mail is strictly prohibited.  If you believe you have received this
> e-mail in error, please immediately delete this e-mail and notify Benjamin
> Bales by telephoning 470-554-2690.
>
> _______________________________________________
> Devel mailing list
> Devel at lists.geany.org
> https://lists.geany.org/cgi-bin/mailman/listinfo/devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.geany.org/pipermail/devel/attachments/20170512/654f96ad/attachment.html>