Re: [Geany-Devel] CodeAi Fixes a Null Pointer Dereference

[Matthew Brush]

On 2017-05-10 04:09 PM, Lex Trotman wrote:

On 11 May 2017 at 08:10, Benjamin Bales 
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@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel

Re: [Geany-Devel] CodeAi Fixes a Null Pointer Dereference

[Matthew Brush]

On 2017-05-10 03:10 PM, Benjamin Bales 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.

diff --git a/plugins/saveactions.c b/plugins/saveactions.c

@@ -280,8 +280,10 @@ static void instantsave_document_new_cb(GObject *obj,
GeanyDocument *doc, gpoint



  doc->file_name = new_filename;



- if (doc->file_type->id == GEANY_FILETYPES_NONE)

+ if(doc->file_type) {

+if (doc->file_type->id == GEANY_FILETYPES_NONE)

  document_set_filetype(doc,
filetypes_lookup_by_name(instantsave_default_ft));

+ }



  /* force saving the file to enable all the related actions(tab name,
filetype, etc.) */

  document_save_file(doc, TRUE);

/* force saving the file to enable all the related actions(tab name,
filetype, etc.) */

document_save_file(doc, TRUE);

}

}

Base-commit: 84253714771f48dbc7fab02f7de43f253734dee2

Please let me know if you are interested in seeing more fixes from our
tool. Thanks!



Hi,

You can submit pull requests with properly formatted changes on Github. 
We've had a few PRs like these where analysis tools were run over the 
codebase and found issues (ex. see PR #166 & #186). If there are 
multiple trivial changes, it's probably fine to put it all in one PR as 
separate commits.


Regards,
Matthew Brush

___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel

Re: [Geany-Devel] CodeAi Fixes a Null Pointer Dereference

[Lex Trotman]

On 11 May 2017 at 08:10, Benjamin Bales 
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.

Cheers
Lex



>
> diff --git a/plugins/saveactions.c b/plugins/saveactions.c
>
> @@ -280,8 +280,10 @@ static void instantsave_document_new_cb(GObject
> *obj, GeanyDocument *doc, gpoint
>
>
>
>   doc->file_name = new_filename;
>
>
>
> - if (doc->file_type->id == GEANY_FILETYPES_NONE)
>
> + if(doc->file_type) {
>
> +if (doc->file_type->id == GEANY_FILETYPES_NONE)
>
>   document_set_filetype(doc, filetypes_lookup_by_name(
> instantsave_default_ft));
>
> + }
>
>
>
>   /* force saving the file to enable all the related actions(tab name,
> filetype, etc.) */
>
>   document_save_file(doc, TRUE);
>
> /* force saving the file to enable all the related actions(tab name,
> filetype, etc.) */
>
> document_save_file(doc, TRUE);
>
> }
>
> }
>
> Base-commit: 84253714771f48dbc7fab02f7de43f253734dee2
>
> Please let me know if you are interested in seeing more fixes from our
> tool. Thanks!
>
> Sincerely,
> --
> 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@lists.geany.org
> https://lists.geany.org/cgi-bin/mailman/listinfo/devel
>
>
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel