Merge branch 'main' into GeekMasher-patch-1

Author: GeekMasher

.gitignore

@@ -7,6 +7,9 @@
 .cache
 *.testproj/
 *.actual
+*.class
+*/*.class
+
 
 # Test files / folders
 test.ql

codeql

@@ -17,40 +17,43 @@ import java
 import semmle.code.java.security.XmlParsers
 import semmle.code.java.dataflow.FlowSources
 import semmle.code.java.dataflow.TaintTracking2
-import DataFlow::PathGraph
+//import DataFlow::PathGraph
 import github.LocalSources
 
-class SafeSAXSourceFlowConfig extends TaintTracking2::Configuration {
-  SafeSAXSourceFlowConfig() { this = "XmlParsers::SafeSAXSourceFlowConfig" }
+module SafeSAXSourceFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSaxSource }
 
-  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSaxSource }
-
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     sink.asExpr() = any(XmlParserCall parse).getSink()
   }
 
-  override int fieldFlowBranchLimit() { result = 0 }
+  int fieldFlowBranchLimit() { result = 0 }
 }
 
+module SafeSAXSourceFlow = TaintTracking::Global<SafeSAXSourceFlowConfig>;
+
 class UnsafeXxeSink extends DataFlow::ExprNode {
   UnsafeXxeSink() {
-    not exists(SafeSAXSourceFlowConfig safeSource | safeSource.hasFlowTo(this)) and
+    not SafeSAXSourceFlow::flowTo(this) and
     exists(XmlParserCall parse |
       parse.getSink() = this.getExpr() and
       not parse.isSafe()
     )
   }
 }
 
-class XxeConfig extends TaintTracking::Configuration {
-  XxeConfig() { this = "XXE.ql::XxeConfig" }
+module XXELocalConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { 
+    source instanceof LocalUserInput and
+    not exists(DataFlow::Node src | src.asExpr() instanceof SafeSaxSource)}
 
-  override predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink }
+  predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink }
 }
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, XxeConfig conf
-where conf.hasFlowPath(source, sink)
+module XXELocalFlow = TaintTracking::Global<XXELocalConfig>;
+import XXELocalFlow::PathGraph
+
+from XXELocalFlow::PathNode source, XXELocalFlow::PathNode sink
+where XXELocalFlow::flowPath(source, sink)
 select sink.getNode(), source, sink, "Unsafe parsing of XML file from $@.", source.getNode(),
   "user input"

java/CWE-611/XXELocal.ql

@@ -0,0 +1,12 @@
+edges
+| XXELocal.java:15:39:15:63 | new FileInputStream(...) : FileInputStream | XXELocal.java:16:51:16:61 | inputStream : FileInputStream |
+| XXELocal.java:16:35:16:62 | new InputSource(...) : InputSource | XXELocal.java:24:25:24:35 | inputSource |
+| XXELocal.java:16:51:16:61 | inputStream : FileInputStream | XXELocal.java:16:35:16:62 | new InputSource(...) : InputSource |
+nodes
+| XXELocal.java:15:39:15:63 | new FileInputStream(...) : FileInputStream | semmle.label | new FileInputStream(...) : FileInputStream |
+| XXELocal.java:16:35:16:62 | new InputSource(...) : InputSource | semmle.label | new InputSource(...) : InputSource |
+| XXELocal.java:16:51:16:61 | inputStream : FileInputStream | semmle.label | inputStream : FileInputStream |
+| XXELocal.java:24:25:24:35 | inputSource | semmle.label | inputSource |
+subpaths
+#select
+| XXELocal.java:24:25:24:35 | inputSource | XXELocal.java:15:39:15:63 | new FileInputStream(...) : FileInputStream | XXELocal.java:24:25:24:35 | inputSource | Unsafe parsing of XML file from $@. | XXELocal.java:15:39:15:63 | new FileInputStream(...) | user input |

tests/java-tests/CWE-611/XXELocal.expected

@@ -0,0 +1,50 @@
+import java.io.File;
+import java.io.FileInputStream;
+import javax.xml.parsers.SAXParser;
+import javax.xml.parsers.SAXParserFactory;
+import org.xml.sax.Attributes;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.helpers.DefaultHandler;
+import org.xml.sax.XMLReader;
+
+public class XXELocal {
+    public static void main(String[] args) throws Exception {
+        // Get user input from file
+        File file = new File("input.xml");
+        FileInputStream inputStream = new FileInputStream(file);
+        InputSource inputSource = new InputSource(inputStream);
+
+        // Get XML reader
+        XMLReader xmlReader = getXMLReader();
+
+        // Parse XML
+        SAXParserFactory saxParserFactory = SAXParserFactory.newInstance();
+        SAXParser saxParser = saxParserFactory.newSAXParser();
+        saxParser.parse(inputSource, new MyHandler());
+    }
+
+    private static XMLReader getXMLReader() throws Exception {
+        SAXParserFactory saxParserFactory = SAXParserFactory.newInstance();
+        SAXParser saxParser = saxParserFactory.newSAXParser();
+        XMLReader xmlReader = saxParser.getXMLReader();
+        return xmlReader;
+    }
+
+    private static class MyHandler extends DefaultHandler {
+        @Override
+        public void startElement(String uri, String localName, String qName, Attributes attributes) throws SAXException {
+            // Handle start element
+        }
+
+        @Override
+        public void endElement(String uri, String localName, String qName) throws SAXException {
+            // Handle end element
+        }
+
+        @Override
+        public void characters(char[] ch, int start, int length) throws SAXException {
+            // Handle character data
+        }
+    }
+}

tests/java-tests/CWE-611/XXELocal.java

@@ -0,0 +1 @@
+CWE-611/XXELocal.ql