From e2dd1269629443fab009b8e9e050005b474ea6e7 Mon Sep 17 00:00:00 2001 From: Porcupiney Hairs Date: Fri, 21 Jun 2024 15:01:40 +0530 Subject: [PATCH 1/6] Python: Pycurl SSL Disabled --- .../lib/semmle/python/frameworks/Pycurl.qll | 45 ++++++++++++++++++- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/python/ql/lib/semmle/python/frameworks/Pycurl.qll b/python/ql/lib/semmle/python/frameworks/Pycurl.qll index 10e534821b6e..e5792976e817 100644 --- a/python/ql/lib/semmle/python/frameworks/Pycurl.qll +++ b/python/ql/lib/semmle/python/frameworks/Pycurl.qll @@ -37,6 +37,15 @@ module Pycurl { /** Gets a reference to an instance of `pycurl.Curl`. */ private API::Node instance() { result = classRef().getReturn() } + /** Gets a reference to an instance of `pycurl.Curl.setopt`. */ + private API::Node setopt() { result = instance().getMember("setopt") } + + /** Gets a reference to an instance of `pycurl.Curl.SSL_VERIFYPEER`. */ + private API::Node sslverifypeer() { + result = API::moduleImport("pycurl").getMember("SSL_VERIFYPEER") or + result = instance().getMember("SSL_VERIFYPEER") + } + /** * When the first parameter value of the `setopt` function is set to `pycurl.URL`, * the second parameter value is the request resource link. @@ -45,7 +54,7 @@ module Pycurl { */ private class OutgoingRequestCall extends Http::Client::Request::Range, DataFlow::CallCfgNode { OutgoingRequestCall() { - this = instance().getMember("setopt").getACall() and + this = setopt().getACall() and this.getArg(0).asCfgNode().(AttrNode).getName() = "URL" } @@ -58,9 +67,41 @@ module Pycurl { override predicate disablesCertificateValidation( DataFlow::Node disablingNode, DataFlow::Node argumentOrigin ) { - // TODO: Look into disabling certificate validation none() } } + + /** + * When the first parameter value of the `setopt` function is set to `SSL_VERIFYPEER` or `SSL_VERIFYHOST`, + * the second parameter value disables or enable SSL certifiacte verification. + * + * See http://pycurl.io/docs/latest/curlobject.html#pycurl.Curl.setopt. + */ + private class CurlSslCall extends Http::Client::Request::Range, DataFlow::CallCfgNode { + CurlSslCall() { + this = setopt().getACall() and + this.getArg(0).asCfgNode().(AttrNode).getName() = ["SSL_VERIFYPEER", "SSL_VERIFYHOST"] + } + + override DataFlow::Node getAUrlPart() { none() } + + override string getFramework() { result = "pycurl.Curl" } + + override predicate disablesCertificateValidation( + DataFlow::Node disablingNode, DataFlow::Node argumentOrigin + ) { + exists(API::CallNode c | + c = setopt().getACall() and + sslverifypeer().getAValueReachableFromSource() = c.getArg(0) and + ( + exists(IntegerLiteral i | i.getValue() = 0 and c.getArg(1).asExpr() = i) + or + exists(BooleanLiteral b | b.booleanValue() = false) + ) + | + disablingNode = c and argumentOrigin = c.getArg(1) + ) + } + } } } From 7ef2d79b3f50e638a4188e3853b4e8ffb2978cdf Mon Sep 17 00:00:00 2001 From: Porcupiney Hairs Date: Mon, 21 Oct 2024 03:28:19 +0530 Subject: [PATCH 2/6] Include changes from review --- .../ql/lib/semmle/python/frameworks/Pycurl.qll | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/python/ql/lib/semmle/python/frameworks/Pycurl.qll b/python/ql/lib/semmle/python/frameworks/Pycurl.qll index e5792976e817..fe052b2caae6 100644 --- a/python/ql/lib/semmle/python/frameworks/Pycurl.qll +++ b/python/ql/lib/semmle/python/frameworks/Pycurl.qll @@ -90,17 +90,13 @@ module Pycurl { override predicate disablesCertificateValidation( DataFlow::Node disablingNode, DataFlow::Node argumentOrigin ) { - exists(API::CallNode c | - c = setopt().getACall() and - sslverifypeer().getAValueReachableFromSource() = c.getArg(0) and - ( - exists(IntegerLiteral i | i.getValue() = 0 and c.getArg(1).asExpr() = i) - or - exists(BooleanLiteral b | b.booleanValue() = false) - ) - | - disablingNode = c and argumentOrigin = c.getArg(1) - ) + sslverifypeer().getAValueReachableFromSource() = this.getArg(0) and + ( + exists(IntegerLiteral i | i.getValue() = 0 and this.getArg(1).asExpr() = i) + or + exists(BooleanLiteral b | b.booleanValue() = false and this.getArg(_).asExpr() = b) + ) and + (disablingNode = this and argumentOrigin = this.getArg(1)) } } } From f6369a6ed73ad14c56191283ec445e18ee3ffc7a Mon Sep 17 00:00:00 2001 From: Porcupiney Hairs Date: Mon, 21 Oct 2024 20:01:44 +0530 Subject: [PATCH 3/6] Include changes from review --- python/ql/lib/semmle/python/frameworks/Pycurl.qll | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/ql/lib/semmle/python/frameworks/Pycurl.qll b/python/ql/lib/semmle/python/frameworks/Pycurl.qll index fe052b2caae6..4c9d53f6dbef 100644 --- a/python/ql/lib/semmle/python/frameworks/Pycurl.qll +++ b/python/ql/lib/semmle/python/frameworks/Pycurl.qll @@ -92,9 +92,9 @@ module Pycurl { ) { sslverifypeer().getAValueReachableFromSource() = this.getArg(0) and ( - exists(IntegerLiteral i | i.getValue() = 0 and this.getArg(1).asExpr() = i) + this.getArg(1).asExpr().(IntegerLiteral).getValue() = 0 or - exists(BooleanLiteral b | b.booleanValue() = false and this.getArg(_).asExpr() = b) + this.getArg(1).asExpr().(BooleanLiteral).booleanValue() = false ) and (disablingNode = this and argumentOrigin = this.getArg(1)) } From c93f0ed851331536ace3aa0e7074db5e9ca45953 Mon Sep 17 00:00:00 2001 From: Porcupiney Hairs Date: Mon, 21 Oct 2024 20:12:46 +0530 Subject: [PATCH 4/6] Include change-note --- .../ql/src/change-notes/change-note-2024-10-21-pyloadSsl.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 python/ql/src/change-notes/change-note-2024-10-21-pyloadSsl.md diff --git a/python/ql/src/change-notes/change-note-2024-10-21-pyloadSsl.md b/python/ql/src/change-notes/change-note-2024-10-21-pyloadSsl.md new file mode 100644 index 000000000000..aa558886f15c --- /dev/null +++ b/python/ql/src/change-notes/change-note-2024-10-21-pyloadSsl.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Improved modelling for the `pycurl` framework. \ No newline at end of file From c7610b3539c9b1885c091f63d065e27e3e9b3410 Mon Sep 17 00:00:00 2001 From: Porcupiney Hairs Date: Mon, 21 Oct 2024 20:14:58 +0530 Subject: [PATCH 5/6] Include change-note --- ...hange-note-2024-10-21-pyloadSsl.md => 2024-10-21-pyloadSsl.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename python/ql/src/change-notes/{change-note-2024-10-21-pyloadSsl.md => 2024-10-21-pyloadSsl.md} (100%) diff --git a/python/ql/src/change-notes/change-note-2024-10-21-pyloadSsl.md b/python/ql/src/change-notes/2024-10-21-pyloadSsl.md similarity index 100% rename from python/ql/src/change-notes/change-note-2024-10-21-pyloadSsl.md rename to python/ql/src/change-notes/2024-10-21-pyloadSsl.md From c78aeec2ec7a046b9007f4b6810d3570d0fba020 Mon Sep 17 00:00:00 2001 From: yoff Date: Thu, 24 Oct 2024 11:44:16 +0200 Subject: [PATCH 6/6] Update python/ql/lib/semmle/python/frameworks/Pycurl.qll --- python/ql/lib/semmle/python/frameworks/Pycurl.qll | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/ql/lib/semmle/python/frameworks/Pycurl.qll b/python/ql/lib/semmle/python/frameworks/Pycurl.qll index 4c9d53f6dbef..9ce97388662a 100644 --- a/python/ql/lib/semmle/python/frameworks/Pycurl.qll +++ b/python/ql/lib/semmle/python/frameworks/Pycurl.qll @@ -37,10 +37,10 @@ module Pycurl { /** Gets a reference to an instance of `pycurl.Curl`. */ private API::Node instance() { result = classRef().getReturn() } - /** Gets a reference to an instance of `pycurl.Curl.setopt`. */ + /** Gets a reference to `pycurl.Curl.setopt`. */ private API::Node setopt() { result = instance().getMember("setopt") } - /** Gets a reference to an instance of `pycurl.Curl.SSL_VERIFYPEER`. */ + /** Gets a reference to the constant `pycurl.Curl.SSL_VERIFYPEER`. */ private API::Node sslverifypeer() { result = API::moduleImport("pycurl").getMember("SSL_VERIFYPEER") or result = instance().getMember("SSL_VERIFYPEER")