From 73d306c8c8fd3fa82dc838b2839560502114b030 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Fri, 17 May 2024 17:34:47 +0200 Subject: [PATCH 1/4] Adds another rule for null deref --- .../Likely Bugs/DerefNullResult.cpp | 23 ++++++++++++++ .../Likely Bugs/DerefNullResult.qhelp | 26 ++++++++++++++++ .../Likely Bugs/DerefNullResult.ql | 31 +++++++++++++++++++ 3 files changed, 80 insertions(+) create mode 100644 cpp/ql/src/experimental/Likely Bugs/DerefNullResult.cpp create mode 100644 cpp/ql/src/experimental/Likely Bugs/DerefNullResult.qhelp create mode 100644 cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql diff --git a/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.cpp b/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.cpp new file mode 100644 index 000000000000..f96d67517b75 --- /dev/null +++ b/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.cpp @@ -0,0 +1,23 @@ +char * create (int arg) { + if (arg > 42) { + // this function may return NULL + return NULL; + } + char * r = malloc(arg); + snprintf(r, arg -1, "Hello"); + return r; +} + +void process(char *str) { + // str is dereferenced + if (str[0] == 'H') { + printf("Hello H\n"); + } +} + +void test(int arg) { + // first function returns a pointer that may be NULL + char *str = create(arg); + // str is not checked for nullness before being passed to process function + process(str); +} diff --git a/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.qhelp b/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.qhelp new file mode 100644 index 000000000000..6c802a9b60b3 --- /dev/null +++ b/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.qhelp @@ -0,0 +1,26 @@ + + + + +

This rule finds a dereference of a function parameter, whose value comes from another function call that may return NULL, without checks in the meantime.

+ + + +

A check should be added between the return of the function which may return NULL, and its use by the function dereferencing ths pointer.

+ + + + + + + +
  • + + Null Dereference + +
    • +     + +     diff --git a/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql b/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql new file mode 100644 index 000000000000..880e5e5c5452 --- /dev/null +++ b/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql @@ -0,0 +1,31 @@ +/** + * @name Null dereference from a function result + * @description A function parameter is dereference, + * while it comes from a function that may return NULL, + * and is not checked for nullness by the caller. + * @kind problem + * @id cpp/deref-null-result + * @problem.severity recommendation + * @tags reliability + * security + * external/cwe/cwe-476 + */ + +import cpp +import semmle.code.cpp.dataflow.DataFlow + +from Function nuller, Parameter pd, FunctionCall fc, Variable v +where + mayReturnNull(nuller) and + functionDereferences(pd.getFunction(), pd.getIndex()) and + + // there is a function call which will deref parameter pd + fc.getTarget() = pd.getFunction() and + // the parameter pd comes from a variable v + DataFlow::localFlow(DataFlow::exprNode(v.getAnAccess()), DataFlow::exprNode(fc.getArgument(pd.getIndex()))) and + // this variable v was assigned by a call to the nuller function + v.getAnAssignedValue() = nuller.getACallToThisFunction() and + // this variable v is not accessed for an operation (check for NULLness) + not exists (VariableAccess vc | vc.getTarget() = v and (vc.getParent() instanceof Operation or vc.getParent() instanceof IfStmt)) + +select fc, "This function call may deref $@ when it can be NULL from $@", v, v.getName(), nuller, nuller.getName() From 8ace9da14a9ee331b841e2dfea9253e30dcb71de Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Mon, 20 May 2024 21:31:47 +0200 Subject: [PATCH 2/4] fixup dataflow path and formatting --- .../experimental/Likely Bugs/DerefNullResult.ql | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql b/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql index 880e5e5c5452..e360ff947216 100644 --- a/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql +++ b/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql @@ -12,20 +12,23 @@ */ import cpp -import semmle.code.cpp.dataflow.DataFlow +import semmle.code.cpp.dataflow.new.DataFlow from Function nuller, Parameter pd, FunctionCall fc, Variable v where mayReturnNull(nuller) and functionDereferences(pd.getFunction(), pd.getIndex()) and - // there is a function call which will deref parameter pd fc.getTarget() = pd.getFunction() and // the parameter pd comes from a variable v - DataFlow::localFlow(DataFlow::exprNode(v.getAnAccess()), DataFlow::exprNode(fc.getArgument(pd.getIndex()))) and + DataFlow::localFlow(DataFlow::exprNode(v.getAnAccess()), + DataFlow::exprNode(fc.getArgument(pd.getIndex()))) and // this variable v was assigned by a call to the nuller function v.getAnAssignedValue() = nuller.getACallToThisFunction() and // this variable v is not accessed for an operation (check for NULLness) - not exists (VariableAccess vc | vc.getTarget() = v and (vc.getParent() instanceof Operation or vc.getParent() instanceof IfStmt)) - -select fc, "This function call may deref $@ when it can be NULL from $@", v, v.getName(), nuller, nuller.getName() + not exists(VariableAccess vc | + vc.getTarget() = v and + (vc.getParent() instanceof Operation or vc.getParent() instanceof IfStmt) + ) +select fc, "This function call may deref $@ when it can be NULL from $@", v, v.getName(), nuller, + nuller.getName() From ab4b823c2ed442caa0a4522b76525430925888ed Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Tue, 21 May 2024 22:10:00 +0200 Subject: [PATCH 3/4] fixup unique assignment --- cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql b/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql index e360ff947216..d36897d9bab4 100644 --- a/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql +++ b/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql @@ -24,7 +24,7 @@ where DataFlow::localFlow(DataFlow::exprNode(v.getAnAccess()), DataFlow::exprNode(fc.getArgument(pd.getIndex()))) and // this variable v was assigned by a call to the nuller function - v.getAnAssignedValue() = nuller.getACallToThisFunction() and + unique( | | v.getAnAssignedValue()) = nuller.getACallToThisFunction() and // this variable v is not accessed for an operation (check for NULLness) not exists(VariableAccess vc | vc.getTarget() = v and From eda815789b11e9591c8f3004dcd2ad45ee6a1113 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 22 May 2024 11:21:04 +0100 Subject: [PATCH 4/4] Update cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql --- cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql b/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql index d36897d9bab4..875c4f1dd966 100644 --- a/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql +++ b/cpp/ql/src/experimental/Likely Bugs/DerefNullResult.ql @@ -1,6 +1,6 @@ /** * @name Null dereference from a function result - * @description A function parameter is dereference, + * @description A function parameter is dereferenced, * while it comes from a function that may return NULL, * and is not checked for nullness by the caller. * @kind problem