From 880c12bd34e6cf2b4fbbdc3bf2d007f187739d4d Mon Sep 17 00:00:00 2001 From: 4B5F5F4B <4b5f5f4b@gmail.com> Date: Mon, 14 Mar 2022 09:42:40 +0800 Subject: [PATCH 1/9] Create CVE --- cpp/ql/src/experimental/Security/CVE | 1 + 1 file changed, 1 insertion(+) create mode 100644 cpp/ql/src/experimental/Security/CVE diff --git a/cpp/ql/src/experimental/Security/CVE b/cpp/ql/src/experimental/Security/CVE new file mode 100644 index 000000000000..8b137891791f --- /dev/null +++ b/cpp/ql/src/experimental/Security/CVE @@ -0,0 +1 @@ + From 4030561eb7c1fdd81322847d1728741c59f7b24b Mon Sep 17 00:00:00 2001 From: 4B5F5F4B <4b5f5f4b@gmail.com> Date: Mon, 14 Mar 2022 09:43:04 +0800 Subject: [PATCH 2/9] Delete CVE --- cpp/ql/src/experimental/Security/CVE | 1 - 1 file changed, 1 deletion(-) delete mode 100644 cpp/ql/src/experimental/Security/CVE diff --git a/cpp/ql/src/experimental/Security/CVE b/cpp/ql/src/experimental/Security/CVE deleted file mode 100644 index 8b137891791f..000000000000 --- a/cpp/ql/src/experimental/Security/CVE +++ /dev/null @@ -1 +0,0 @@ - From 597603a3a6e3ec795a1e47a99eea29f9dca98061 Mon Sep 17 00:00:00 2001 From: 4B5F5F4B <4b5f5f4b@gmail.com> Date: Mon, 14 Mar 2022 09:44:30 +0800 Subject: [PATCH 3/9] Create cve-2017-5123.ql Add query to detect CVE-2017-5123 --- .../Security/CVE/cve-2017-5123.ql | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 cpp/ql/src/experimental/Security/CVE/cve-2017-5123.ql diff --git a/cpp/ql/src/experimental/Security/CVE/cve-2017-5123.ql b/cpp/ql/src/experimental/Security/CVE/cve-2017-5123.ql new file mode 100644 index 000000000000..197359ca5c3c --- /dev/null +++ b/cpp/ql/src/experimental/Security/CVE/cve-2017-5123.ql @@ -0,0 +1,49 @@ +import cpp +import semmle.code.cpp.dataflow.DataFlow + + +class WrtieAccessCheckMacro extends Macro{ + VariableAccess va; + WrtieAccessCheckMacro(){ + this.getName() = ["user_write_access_begin", + "user_access_begin"] + and + va.getEnclosingElement() = this.getAnInvocation().getAnExpandedElement() + } + + VariableAccess getArgument(){ + result = va + } +} + + +class UnSafePutUserMacro extends Macro{ + PointerDereferenceExpr writeUserPtr; + + UnSafePutUserMacro(){ + this.getName() = "unsafe_put_user" and + writeUserPtr.getEnclosingElement() = this.getAnInvocation().getAnExpandedElement() + } + + Expr getUserModePtr(){ + result = writeUserPtr.getOperand().(AddressOfExpr).getOperand().(FieldAccess).getQualifier() + } +} + +class ExploitableUserModePtrParam extends Parameter{ + ExploitableUserModePtrParam(){ + not exists(WrtieAccessCheckMacro writeAccessCheck| + DataFlow::localFlow(DataFlow::parameterNode(this), DataFlow::exprNode(writeAccessCheck.getArgument())) + ) + } +} + + +from ExploitableUserModePtrParam p, UnSafePutUserMacro unsafePutUser +where + DataFlow::localFlow(DataFlow::parameterNode(p), DataFlow::exprNode(unsafePutUser.getUserModePtr())) +select + p, unsafePutUser, "potential wrtie user mode ptr without check." + + + From baf1c8d76b499506c15d21bd1a1aa7e50296bcfd Mon Sep 17 00:00:00 2001 From: 4B5F5F4B <4b5f5f4b@gmail.com> Date: Wed, 16 Mar 2022 17:49:05 +0800 Subject: [PATCH 4/9] Create cve-2016-6480.ql --- .../Security/CVE/cve-2016-6480.ql | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 cpp/ql/src/experimental/Security/CVE/cve-2016-6480.ql diff --git a/cpp/ql/src/experimental/Security/CVE/cve-2016-6480.ql b/cpp/ql/src/experimental/Security/CVE/cve-2016-6480.ql new file mode 100644 index 000000000000..dccf036ceded --- /dev/null +++ b/cpp/ql/src/experimental/Security/CVE/cve-2016-6480.ql @@ -0,0 +1,34 @@ +import cpp + +class CopyFromUserFunctionCall extends FunctionCall{ + CopyFromUserFunctionCall(){ + this.getTarget().getName() = "copy_from_user" + and not this.getArgument(1) instanceof AddressOfExpr + } + + predicate hasSameArguments(CopyFromUserFunctionCall another){ + this.getArgument(0).toString() = another.getArgument(0).toString() + and this.getArgument(1).toString() = another.getArgument(1).toString() + } + +} + +from CopyFromUserFunctionCall p1, CopyFromUserFunctionCall p2 +where + not p1 = p2 + and p1.hasSameArguments(p2) + and exists(IfStmt ifStmt| + p1.getBasicBlock().getAFalseSuccessor*() = ifStmt.getBasicBlock() + and ifStmt.getBasicBlock().getAFalseSuccessor*() = p2.getBasicBlock() + ) + and not exists(AssignPointerAddExpr assignPtrAdd | + p1.getArgument(1).toString() = assignPtrAdd.getLValue().toString() + and p1.getBasicBlock().getAFalseSuccessor*() = assignPtrAdd.getBasicBlock() + ) +select + "first fetch", p1, "double fetch", p2 + + + + + From d4c73144843a07d7834b924ae18f66c33d783e8e Mon Sep 17 00:00:00 2001 From: 4B5F5F4B <4b5f5f4b@gmail.com> Date: Thu, 17 Mar 2022 09:49:28 +0800 Subject: [PATCH 5/9] Delete cve-2016-6480.ql commit by mistake --- .../Security/CVE/cve-2016-6480.ql | 34 ------------------- 1 file changed, 34 deletions(-) delete mode 100644 cpp/ql/src/experimental/Security/CVE/cve-2016-6480.ql diff --git a/cpp/ql/src/experimental/Security/CVE/cve-2016-6480.ql b/cpp/ql/src/experimental/Security/CVE/cve-2016-6480.ql deleted file mode 100644 index dccf036ceded..000000000000 --- a/cpp/ql/src/experimental/Security/CVE/cve-2016-6480.ql +++ /dev/null @@ -1,34 +0,0 @@ -import cpp - -class CopyFromUserFunctionCall extends FunctionCall{ - CopyFromUserFunctionCall(){ - this.getTarget().getName() = "copy_from_user" - and not this.getArgument(1) instanceof AddressOfExpr - } - - predicate hasSameArguments(CopyFromUserFunctionCall another){ - this.getArgument(0).toString() = another.getArgument(0).toString() - and this.getArgument(1).toString() = another.getArgument(1).toString() - } - -} - -from CopyFromUserFunctionCall p1, CopyFromUserFunctionCall p2 -where - not p1 = p2 - and p1.hasSameArguments(p2) - and exists(IfStmt ifStmt| - p1.getBasicBlock().getAFalseSuccessor*() = ifStmt.getBasicBlock() - and ifStmt.getBasicBlock().getAFalseSuccessor*() = p2.getBasicBlock() - ) - and not exists(AssignPointerAddExpr assignPtrAdd | - p1.getArgument(1).toString() = assignPtrAdd.getLValue().toString() - and p1.getBasicBlock().getAFalseSuccessor*() = assignPtrAdd.getBasicBlock() - ) -select - "first fetch", p1, "double fetch", p2 - - - - - From 64863d493b79cde436cf4e38abab5e092bf9e4eb Mon Sep 17 00:00:00 2001 From: 4B5F5F4B <4b5f5f4b@gmail.com> Date: Sat, 26 Mar 2022 22:42:59 +0800 Subject: [PATCH 6/9] Delete cve-2017-5123.ql --- .../Security/CVE/cve-2017-5123.ql | 49 ------------------- 1 file changed, 49 deletions(-) delete mode 100644 cpp/ql/src/experimental/Security/CVE/cve-2017-5123.ql diff --git a/cpp/ql/src/experimental/Security/CVE/cve-2017-5123.ql b/cpp/ql/src/experimental/Security/CVE/cve-2017-5123.ql deleted file mode 100644 index 197359ca5c3c..000000000000 --- a/cpp/ql/src/experimental/Security/CVE/cve-2017-5123.ql +++ /dev/null @@ -1,49 +0,0 @@ -import cpp -import semmle.code.cpp.dataflow.DataFlow - - -class WrtieAccessCheckMacro extends Macro{ - VariableAccess va; - WrtieAccessCheckMacro(){ - this.getName() = ["user_write_access_begin", - "user_access_begin"] - and - va.getEnclosingElement() = this.getAnInvocation().getAnExpandedElement() - } - - VariableAccess getArgument(){ - result = va - } -} - - -class UnSafePutUserMacro extends Macro{ - PointerDereferenceExpr writeUserPtr; - - UnSafePutUserMacro(){ - this.getName() = "unsafe_put_user" and - writeUserPtr.getEnclosingElement() = this.getAnInvocation().getAnExpandedElement() - } - - Expr getUserModePtr(){ - result = writeUserPtr.getOperand().(AddressOfExpr).getOperand().(FieldAccess).getQualifier() - } -} - -class ExploitableUserModePtrParam extends Parameter{ - ExploitableUserModePtrParam(){ - not exists(WrtieAccessCheckMacro writeAccessCheck| - DataFlow::localFlow(DataFlow::parameterNode(this), DataFlow::exprNode(writeAccessCheck.getArgument())) - ) - } -} - - -from ExploitableUserModePtrParam p, UnSafePutUserMacro unsafePutUser -where - DataFlow::localFlow(DataFlow::parameterNode(p), DataFlow::exprNode(unsafePutUser.getUserModePtr())) -select - p, unsafePutUser, "potential wrtie user mode ptr without check." - - - From 7a091f808bb68863af08842155b2ce804015928b Mon Sep 17 00:00:00 2001 From: 4B5F5F4B <4b5f5f4b@gmail.com> Date: Sat, 26 Mar 2022 22:45:03 +0800 Subject: [PATCH 7/9] Create NoCheckBeforeUnsafePutUser.ql --- .../CWE/CWE-020/NoCheckBeforeUnsafePutUser.ql | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-020/NoCheckBeforeUnsafePutUser.ql diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-020/NoCheckBeforeUnsafePutUser.ql b/cpp/ql/src/experimental/Security/CWE/CWE-020/NoCheckBeforeUnsafePutUser.ql new file mode 100644 index 000000000000..77c67ccaebaa --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-020/NoCheckBeforeUnsafePutUser.ql @@ -0,0 +1,56 @@ +/** + * @name Linux kernel no check before unsafe_put_user vulnerability detection + * @description unsafe_put_user which is used to write data to user-mode + * memory is widely used in Linux kernel codebase, but if + * there is no security check for user-mode pointer used as + * parameter of unsafe_put_user, attacker can exploit the issue + * to obtain root privilege. CVE-2017-5123 is quite a good + * example for your information. + * @kind problem + * @id cpp/linux-kernel-no-check-before-unsafe-put-user + * @problem.severity warning + * @security-severity 7.5 + * @tags security + * external/cwe/cwe-020 + */ + +import cpp +import semmle.code.cpp.dataflow.DataFlow + +class WrtieAccessCheckMacro extends Macro { + VariableAccess va; + + WrtieAccessCheckMacro() { + this.getName() = ["user_write_access_begin", "user_access_begin"] and + va.getEnclosingElement() = this.getAnInvocation().getAnExpandedElement() + } + + VariableAccess getArgument() { result = va } +} + +class UnSafePutUserMacro extends Macro { + PointerDereferenceExpr writeUserPtr; + + UnSafePutUserMacro() { + this.getName() = "unsafe_put_user" and + writeUserPtr.getEnclosingElement() = this.getAnInvocation().getAnExpandedElement() + } + + Expr getUserModePtr() { + result = writeUserPtr.getOperand().(AddressOfExpr).getOperand().(FieldAccess).getQualifier() + } +} + +class ExploitableUserModePtrParam extends Parameter { + ExploitableUserModePtrParam() { + not exists(WrtieAccessCheckMacro writeAccessCheck | + DataFlow::localFlow(DataFlow::parameterNode(this), + DataFlow::exprNode(writeAccessCheck.getArgument())) + ) + } +} + +from ExploitableUserModePtrParam p, UnSafePutUserMacro unsafePutUser +where + DataFlow::localFlow(DataFlow::parameterNode(p), DataFlow::exprNode(unsafePutUser.getUserModePtr())) +select p, unsafePutUser, "potential wrtie user mode ptr without check." From 2d7b9c0c4f671b7a745e7d415481bb1783fa6cf8 Mon Sep 17 00:00:00 2001 From: 4B5F5F4B <4b5f5f4b@gmail.com> Date: Sat, 26 Mar 2022 22:55:27 +0800 Subject: [PATCH 8/9] modify a little cute typo --- .../Security/CWE/CWE-020/NoCheckBeforeUnsafePutUser.ql | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-020/NoCheckBeforeUnsafePutUser.ql b/cpp/ql/src/experimental/Security/CWE/CWE-020/NoCheckBeforeUnsafePutUser.ql index 77c67ccaebaa..6bdfe6c84b97 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-020/NoCheckBeforeUnsafePutUser.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-020/NoCheckBeforeUnsafePutUser.ql @@ -17,10 +17,10 @@ import cpp import semmle.code.cpp.dataflow.DataFlow -class WrtieAccessCheckMacro extends Macro { +class WriteAccessCheckMacro extends Macro { VariableAccess va; - WrtieAccessCheckMacro() { + WriteAccessCheckMacro() { this.getName() = ["user_write_access_begin", "user_access_begin"] and va.getEnclosingElement() = this.getAnInvocation().getAnExpandedElement() } @@ -43,7 +43,7 @@ class UnSafePutUserMacro extends Macro { class ExploitableUserModePtrParam extends Parameter { ExploitableUserModePtrParam() { - not exists(WrtieAccessCheckMacro writeAccessCheck | + not exists(WriteAccessCheckMacro writeAccessCheck | DataFlow::localFlow(DataFlow::parameterNode(this), DataFlow::exprNode(writeAccessCheck.getArgument())) ) From 9358b824c03a087e8c4b0c78aa6339b679e7150a Mon Sep 17 00:00:00 2001 From: 4B5F5F4B <4b5f5f4b@gmail.com> Date: Tue, 29 Mar 2022 10:41:12 +0800 Subject: [PATCH 9/9] modify select clause to make codeql happy:) --- .../Security/CWE/CWE-020/NoCheckBeforeUnsafePutUser.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-020/NoCheckBeforeUnsafePutUser.ql b/cpp/ql/src/experimental/Security/CWE/CWE-020/NoCheckBeforeUnsafePutUser.ql index 6bdfe6c84b97..ed43419e3298 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-020/NoCheckBeforeUnsafePutUser.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-020/NoCheckBeforeUnsafePutUser.ql @@ -53,4 +53,4 @@ class ExploitableUserModePtrParam extends Parameter { from ExploitableUserModePtrParam p, UnSafePutUserMacro unsafePutUser where DataFlow::localFlow(DataFlow::parameterNode(p), DataFlow::exprNode(unsafePutUser.getUserModePtr())) -select p, unsafePutUser, "potential wrtie user mode ptr without check." +select p, "unsafe_put_user write user-mode pointer $@ without check.", p, p.toString()