[Geany-Devel] CodeAi Fixes a Null Pointer Dereference

[Benjamin Bales]

ok, I'll submit a PR with some of the fixes that I think are good.  Was the
format of the bug report acceptable?  Our tool integrates with static
analyzers, and sometimes its tricky to include their reports in a way that
is clear and concise.  Of course, I will provide my own summary of the
issues, but I would like to know if you found them helpful.

-Ben

On Thu, May 11, 2017 at 4:24 PM, Matthew Brush <mbrush at codebrainz.ca> wrote:

> On 2017-05-11 12:40 PM, Benjamin Bales 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.
>>
>>
> If they're valid bugs (even minor), it certainly doesn't hurt to make a PR
> (or even an Issue just giving the info). They'll probably require more than
> an automated fix though, to account for formatting/indentation as well as
> looking at how it affects the code. Likely they'll need a human to evaluate
> what the correct fix is, for example the one given before is probably not
> very good as it just guards out code that was expected to run, rather than
> figuring out how the code was meant to work and make it do that.
>
> Regards,
> Matthew Brush
>
>
> -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
>>>
>>>
>>
>>
>>
>>
>> _______________________________________________
>> Devel mailing list
>> Devel at lists.geany.org
>> https://lists.geany.org/cgi-bin/mailman/listinfo/devel
>>
>>
> _______________________________________________
> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.geany.org/pipermail/devel/attachments/20170511/bac7a808/attachment-0001.html>