From f2f0873d911dc9bb685fa708707e3f4c1de6fc9d Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Wed, 6 Apr 2022 15:49:06 +0200
Subject: [PATCH] Python: Use new `API::CallNode` for XML constant check

This also means that the detection of the values passed to these keyword
arguments will no longer just be from a local scope, but can also be
across function boundaries.
---
 .../ql/lib/semmle/python/frameworks/Lxml.qll  | 21 ++++++++++---------
 .../semmle/python/frameworks/Xmltodict.qll    |  4 ++--
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/python/ql/lib/semmle/python/frameworks/Lxml.qll b/python/ql/lib/semmle/python/frameworks/Lxml.qll
index 24afbd199df..a77da9e7915 100644
--- a/python/ql/lib/semmle/python/frameworks/Lxml.qll
+++ b/python/ql/lib/semmle/python/frameworks/Lxml.qll
@@ -129,7 +129,7 @@ private module Lxml {
      *
      * See https://lxml.de/apidoc/lxml.etree.html?highlight=xmlparser#lxml.etree.XMLParser
      */
-    private class LxmlParser extends InstanceSource, DataFlow::CallCfgNode {
+    private class LxmlParser extends InstanceSource, API::CallNode {
       LxmlParser() {
         this = API::moduleImport("lxml").getMember("etree").getMember("XMLParser").getACall()
       }
@@ -141,16 +141,17 @@ private module Lxml {
           // resolve_entities has default True
           not exists(this.getArgByName("resolve_entities"))
           or
-          this.getArgByName("resolve_entities").getALocalSource().asExpr() = any(True t)
+          this.getKeywordParameter("resolve_entities").getAValueReachingRhs().asExpr() = any(True t)
         )
         or
         (kind.isBillionLaughs() or kind.isQuadraticBlowup()) and
-        this.getArgByName("huge_tree").getALocalSource().asExpr() = any(True t) and
-        not this.getArgByName("resolve_entities").getALocalSource().asExpr() = any(False t)
+        this.getKeywordParameter("huge_tree").getAValueReachingRhs().asExpr() = any(True t) and
+        not this.getKeywordParameter("resolve_entities").getAValueReachingRhs().asExpr() =
+          any(False t)
         or
         kind.isDtdRetrieval() and
-        this.getArgByName("load_dtd").getALocalSource().asExpr() = any(True t) and
-        this.getArgByName("no_network").getALocalSource().asExpr() = any(False t)
+        this.getKeywordParameter("load_dtd").getAValueReachingRhs().asExpr() = any(True t) and
+        this.getKeywordParameter("no_network").getAValueReachingRhs().asExpr() = any(False t)
       }
     }
 
@@ -305,7 +306,7 @@ private module Lxml {
    * See
    * - https://lxml.de/apidoc/lxml.etree.html?highlight=parseids#lxml.etree.iterparse
    */
-  private class LxmlIterparseCall extends DataFlow::CallCfgNode, XML::XmlParsing::Range,
+  private class LxmlIterparseCall extends API::CallNode, XML::XmlParsing::Range,
     FileSystemAccess::Range {
     LxmlIterparseCall() {
       this = API::moduleImport("lxml").getMember("etree").getMember("iterparse").getACall()
@@ -318,11 +319,11 @@ private module Lxml {
       kind.isXxe()
       or
       (kind.isBillionLaughs() or kind.isQuadraticBlowup()) and
-      this.getArgByName("huge_tree").getALocalSource().asExpr() = any(True t)
+      this.getKeywordParameter("huge_tree").getAValueReachingRhs().asExpr() = any(True t)
       or
       kind.isDtdRetrieval() and
-      this.getArgByName("load_dtd").getALocalSource().asExpr() = any(True t) and
-      this.getArgByName("no_network").getALocalSource().asExpr() = any(False t)
+      this.getKeywordParameter("load_dtd").getAValueReachingRhs().asExpr() = any(True t) and
+      this.getKeywordParameter("no_network").getAValueReachingRhs().asExpr() = any(False t)
     }
 
     override predicate mayExecuteInput() { none() }
diff --git a/python/ql/lib/semmle/python/frameworks/Xmltodict.qll b/python/ql/lib/semmle/python/frameworks/Xmltodict.qll
index db2c443d8e9..95d44d6d1b0 100644
--- a/python/ql/lib/semmle/python/frameworks/Xmltodict.qll
+++ b/python/ql/lib/semmle/python/frameworks/Xmltodict.qll
@@ -20,7 +20,7 @@ private module Xmltodict {
   /**
    * A call to `xmltodict.parse`.
    */
-  private class XMLtoDictParsing extends DataFlow::CallCfgNode, XML::XmlParsing::Range {
+  private class XMLtoDictParsing extends API::CallNode, XML::XmlParsing::Range {
     XMLtoDictParsing() { this = API::moduleImport("xmltodict").getMember("parse").getACall() }
 
     override DataFlow::Node getAnInput() {
@@ -29,7 +29,7 @@ private module Xmltodict {
 
     override predicate vulnerableTo(XML::XmlParsingVulnerabilityKind kind) {
       (kind.isBillionLaughs() or kind.isQuadraticBlowup()) and
-      this.getArgByName("disable_entities").getALocalSource().asExpr() = any(False f)
+      this.getKeywordParameter("disable_entities").getAValueReachingRhs().asExpr() = any(False f)
     }
 
     override predicate mayExecuteInput() { none() }