Merge pull request #13713 from pwntester/java/struts2_source_taint_inheriting

[Java] Implement field taint inheritance for Struts2 unmarshalled objects

Author: atorralba

java/ql/lib/change-notes/2023-07-12-improve-sources-for-the-struts2-framework.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+* Improved the modeling of Struts 2 sources of untrusted data by tainting the whole object graph of the objects unmarshaled from an HTTP request.
+

java/ql/lib/semmle/code/java/Serializability.qll

@@ -6,6 +6,7 @@ import java
 private import frameworks.jackson.JacksonSerializability
 private import frameworks.google.GsonSerializability
 private import frameworks.google.GoogleHttpClientApi
+private import frameworks.struts.Struts2Serializability
 
 /**
  * A serializable field may be read without code referencing it,

java/ql/lib/semmle/code/java/dataflow/FlowSources.qll

@@ -143,11 +143,10 @@ private class GuiceRequestParameterSource extends RemoteFlowSource {
   override string getSourceType() { result = "Guice request parameter" }
 }
 
-private class Struts2ActionSupportClassFieldReadSource extends RemoteFlowSource {
-  Struts2ActionSupportClassFieldReadSource() {
-    exists(Struts2ActionSupportClass c |
-      c.getASetterMethod().getField() = this.asExpr().(FieldRead).getField()
-    )
+private class Struts2ActionSupportClassFieldSource extends RemoteFlowSource {
+  Struts2ActionSupportClassFieldSource() {
+    this.(DataFlow::FieldValueNode).getField() =
+      any(Struts2ActionSupportClass c).getASetterMethod().getField()
   }
 
   override string getSourceType() { result = "Struts2 ActionSupport field" }

java/ql/lib/semmle/code/java/frameworks/struts/Struts2Serializability.qll

@@ -0,0 +1,52 @@
+/**
+ * Provides classes and predicates for working with objects bound from Http requests in the context of
+ * the Struts2 web framework.
+ */
+
+import java
+private import semmle.code.java.Serializability
+private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.dataflow.FlowSteps
+private import semmle.code.java.frameworks.struts.StrutsActions
+
+/** A type whose values may be unmarshalled from an Http request by the Struts2 framework. */
+abstract class Struts2DeserializableType extends Type { }
+
+/** A type whose values are explicitly unmarshalled by from an Http request by the Struts2 framework. */
+private class ExplicitlyReadStruts2DeserializableType extends Struts2DeserializableType {
+  ExplicitlyReadStruts2DeserializableType() {
+    exists(Struts2ActionSupportClass c |
+      usesType(c.getASetterMethod().getField().getType(), this) and
+      not this instanceof TypeClass and
+      not this instanceof TypeObject
+    )
+  }
+}
+
+/** A type used in a `Struts2ActionField` declaration. */
+private class FieldReferencedStruts2DeserializableType extends Struts2DeserializableType {
+  FieldReferencedStruts2DeserializableType() {
+    exists(Struts2ActionField f | usesType(f.getType(), this))
+  }
+}
+
+/** A field that may be unmarshalled from an Http request using the Struts2 framework. */
+private class Struts2ActionField extends DeserializableField {
+  Struts2ActionField() {
+    exists(Struts2DeserializableType superType |
+      superType = this.getDeclaringType().getAnAncestor() and
+      not superType instanceof TypeObject and
+      superType.fromSource() and
+      (
+        this.isPublic()
+        or
+        exists(SetterMethod setter | setter.getField() = this and setter.isPublic())
+      )
+    )
+  }
+}
+
+/** A field that should convey the taint from its qualifier to itself. */
+private class Struts2ActionFieldInheritTaint extends DataFlow::FieldContent, TaintInheritingContent {
+  Struts2ActionFieldInheritTaint() { this.getField() instanceof Struts2ActionField }
+}