Merge pull request #13256 from atorralba/atorralba/java/stapler-models

Java: Model the Stapler framework

java/ql/lib/change-notes/2023-05-22-stapler-models.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added more models for the Stapler framework.

java/ql/lib/ext/org.kohsuke.stapler.bind.model.yml

@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sourceModel
+    data:
+      - ["org.kohsuke.stapler.bind", "JavaScriptMethod", True, "", "", "Annotated", "Parameter", "remote", "manual"]

java/ql/lib/ext/org.kohsuke.stapler.json.model.yml

@@ -0,0 +1,7 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sourceModel
+    data:
+      - ["org.kohsuke.stapler.json", "SubmittedForm", True, "", "", "Annotated", "Parameter", "remote", "manual"]
+      - ["org.kohsuke.stapler.json", "JsonBody", True, "", "", "Annotated", "Parameter", "remote", "manual"]

java/ql/lib/ext/org.kohsuke.stapler.model.yml

@@ -4,4 +4,46 @@ extensions:
       extensible: sinkModel
     data:
       - ["org.kohsuke.stapler", "HttpResponses", True, "redirectTo", "(String)", "", "Argument[0]", "url-redirection", "ai-manual"]
+      - ["org.kohsuke.stapler", "HttpResponses", True, "redirectTo", "(int,String)", "", "Argument[1]", "url-redirection", "manual"]
       - ["org.kohsuke.stapler", "HttpResponses", True, "staticResource", "(URL)", "", "Argument[0]", "request-forgery", "ai-manual"]
+      - ["org.kohsuke.stapler", "HttpResponses", True, "staticResource", "(URL,long)", "", "Argument[0]", "request-forgery", "manual"]
+      - ["org.kohsuke.stapler", "HttpResponses", True, "html", "(String)", "", "Argument[0]", "html-injection", "manual"]
+      - ["org.kohsuke.stapler", "HttpResponses", True, "literalHtml", "(String)", "", "Argument[0]", "html-injection", "manual"]
+      - ["org.kohsuke.stapler", "StaplerResponse", True, "forward", "(Object,String,StaplerRequest)", "", "Argument[1]", "request-forgery", "manual"]
+      - ["org.kohsuke.stapler", "StaplerResponse", True, "sendRedirect2", "(String)", "", "Argument[0]", "url-redirection", "manual"]
+      - ["org.kohsuke.stapler", "StaplerResponse", True, "sendRedirect", "(int,String)", "", "Argument[1]", "url-redirection", "manual"]
+      - ["org.kohsuke.stapler", "StaplerResponse", True, "sendRedirect", "(String)", "", "Argument[0]", "url-redirection", "manual"]
+      - ["org.kohsuke.stapler", "StaplerResponse", True, "serveFile", "(StaplerRequest,URL)", "", "Argument[1]", "path-injection", "manual"]
+      - ["org.kohsuke.stapler", "StaplerResponse", True, "serveFile", "(StaplerRequest,URL,long)", "", "Argument[1]", "path-injection", "manual"]
+      - ["org.kohsuke.stapler", "StaplerResponse", True, "serveLocalizedFile", "(StaplerRequest,URL)", "", "Argument[1]", "path-injection", "manual"]
+      - ["org.kohsuke.stapler", "StaplerResponse", True, "serveLocalizedFile", "(StaplerRequest,URL,long)", "", "Argument[1]", "path-injection", "manual"]
+      - ["org.kohsuke.stapler", "StaplerResponse", True, "serveFile", "(StaplerRequest,InputStream,long,long,long,String)", "", "Argument[1]", "path-injection", "manual"]
+      - ["org.kohsuke.stapler", "StaplerResponse", True, "serveFile", "(StaplerRequest,InputStream,long,long,int,String)", "", "Argument[1]", "path-injection", "manual"]
+      - ["org.kohsuke.stapler", "StaplerResponse", True, "serveFile", "(StaplerRequest,InputStream,long,long,String)", "", "Argument[1]", "path-injection", "manual"]
+      - ["org.kohsuke.stapler", "StaplerResponse", True, "serveFile", "(StaplerRequest,InputStream,long,int,String)", "", "Argument[1]", "path-injection", "manual"]
+      - ["org.kohsuke.stapler", "StaplerResponse", True, "reverseProxyTo", "(URL,StaplerRequest)", "", "Argument[0]", "request-forgery", "manual"]
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sourceModel
+    data:
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "getRequestURIWithQueryString", "", "", "ReturnValue", "remote", "manual"]
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "getRequestURLWithQueryString", "", "", "ReturnValue", "remote", "manual"]
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "getReferer", "", "", "ReturnValue", "remote", "manual"]
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "getOriginalRequestURI", "", "", "ReturnValue", "remote", "manual"]
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "getSubmittedForm", "", "", "ReturnValue", "remote", "manual"]
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "getFileItem", "", "", "ReturnValue", "remote", "manual"]
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "bindParametersToList", "", "", "ReturnValue", "remote", "manual"]
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "bindParameters", "", "", "Argument[0]", "remote", "manual"]
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "bindParameters", "", "", "ReturnValue", "remote", "manual"]
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "bindJSON", "", "", "Argument[0]", "remote", "manual"]
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "bindJSON", "", "", "ReturnValue", "remote", "manual"]
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "bindJSONToList", "", "", "ReturnValue", "remote", "manual"]
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "getParameter", "", "", "ReturnValue", "remote", "manual"]
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "getParameterMap", "", "", "ReturnValue", "remote", "manual"]
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "getParameterNames", "", "", "ReturnValue", "remote", "manual"]
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "getParameterValues", "", "", "ReturnValue", "remote", "manual"]
+      - ["org.kohsuke.stapler", "StaplerRequest", True, "getRestOfPath", "", "", "ReturnValue", "remote", "manual"]
+      - ["org.kohsuke.stapler", "QueryParameter", True, "", "", "Annotated", "Parameter", "remote", "manual"]
+      - ["org.kohsuke.stapler", "Header", True, "", "", "Annotated", "Parameter", "remote", "manual"]
+      - ["org.kohsuke.stapler", "DataBoundConstructor", True, "", "", "Annotated", "Parameter", "remote", "manual"]
+      - ["org.kohsuke.stapler", "DataBoundSetter", True, "", "", "Annotated", "Parameter", "remote", "manual"]

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

@@ -41,6 +41,7 @@ abstract class RemoteFlowSource extends DataFlow::Node {
  */
 private module FlowSources {
   private import semmle.code.java.frameworks.hudson.Hudson
+  private import semmle.code.java.frameworks.stapler.Stapler
 }
 
 private class ExternalRemoteFlowSource extends RemoteFlowSource {

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

@@ -23,6 +23,7 @@ private module Frameworks {
   private import semmle.code.java.frameworks.Properties
   private import semmle.code.java.frameworks.Protobuf
   private import semmle.code.java.frameworks.ratpack.RatpackExec
+  private import semmle.code.java.frameworks.stapler.Stapler
   private import semmle.code.java.JDK
 }
 

java/ql/lib/semmle/code/java/frameworks/hudson/Hudson.qll

@@ -2,8 +2,17 @@
 
 import java
 private import semmle.code.java.dataflow.FlowSources
+private import semmle.code.java.frameworks.stapler.Stapler
 private import semmle.code.java.security.XSS
 
+/** A method declared in a subtype of `hudson.model.Descriptor` that returns an `HttpResponse`. */
+class HudsonWebMethod extends Method {
+  HudsonWebMethod() {
+    this.getReturnType().(RefType).getASourceSupertype*() instanceof HttpResponse and
+    this.getDeclaringType().getASourceSupertype*().hasQualifiedName("hudson.model", "Descriptor")
+  }
+}
+
 private class FilePathRead extends LocalUserInput {
   FilePathRead() {
     this.asExpr()

java/ql/lib/semmle/code/java/frameworks/stapler/Stapler.qll

@@ -0,0 +1,124 @@
+/** Provides classes and predicates related to the Stapler framework. */
+
+import java
+private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.dataflow.FlowSources
+private import semmle.code.java.dataflow.FlowSteps
+private import semmle.code.java.dataflow.TypeFlow
+private import semmle.code.java.frameworks.hudson.Hudson
+private import semmle.code.java.frameworks.JavaxAnnotations
+
+/**
+ * A callable annotated with a Stapler `DataBound` annotation,
+ * or that has the `@stapler-constructor` Javadoc annotation.
+ */
+class DataBoundAnnotated extends Callable {
+  DataBoundAnnotated() {
+    exists(Annotation an |
+      an.getType()
+          .hasQualifiedName("org.kohsuke.stapler", ["DataBoundConstructor", "DataBoundSetter"])
+    |
+      this = an.getAnnotatedElement()
+    )
+    or
+    exists(Javadoc doc | doc.getAChild().getText().matches("%@stapler-constructor%") |
+      doc.getCommentedElement() = this
+    )
+  }
+}
+
+/** The interface `org.kohsuke.stapler.HttpResponse`. */
+class HttpResponse extends Interface {
+  HttpResponse() { this.hasQualifiedName("org.kohsuke.stapler", "HttpResponse") }
+}
+
+/**
+ * A remote flow source for parameters annotated with an annotation
+ * that is itself annotated with `InjectedParameter`.
+ *
+ * Such parameters are populated with user-provided data by Stapler.
+ */
+private class InjectedParameterSource extends RemoteFlowSource {
+  InjectedParameterSource() {
+    this.asParameter().getAnAnnotation().getType() instanceof InjectedParameterAnnotatedType
+  }
+
+  override string getSourceType() { result = "Stapler injected parameter" }
+}
+
+/**
+ * A dataflow step from the `HttpResponse` return value of a `HudsonWebMethod`
+ * to the instance parameter of the `generateResponse` method of the appropriate subtype of `HttpResponse`.
+ *
+ * This models the rendering process of an `HttpResponse` by Stapler.
+ */
+private class HttpResponseGetDescriptionStep extends AdditionalValueStep {
+  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+    exists(ReturnStmt s, GenerateResponseMethod m |
+      s.getEnclosingCallable() instanceof HudsonWebMethod and
+      boundOrStaticType(s.getResult(), m.getDeclaringType().getADescendant())
+    |
+      n1.asExpr() = s.getResult() and
+      n2.(DataFlow::InstanceParameterNode).getCallable() = m
+    )
+  }
+}
+
+/**
+ * A dataflow step from the post-update node of an instance access in a `DataBoundAnnotated` method
+ * to the instance parameter of a `PostConstruct` method of the same type.
+ *
+ * This models the construction process of a `DataBound` object in Stapler.
+ */
+private class PostConstructDataBoundAdditionalStep extends AdditionalValueStep {
+  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+    exists(PostConstructDataBoundMethod postConstruct, DataBoundAnnotated input |
+      postConstruct.getDeclaringType() = input.getDeclaringType()
+    |
+      n1.(DataFlow::PostUpdateNode)
+          .getPreUpdateNode()
+          .(DataFlow::InstanceAccessNode)
+          .getEnclosingCallable() = input and
+      n2.(DataFlow::InstanceParameterNode).getCallable() = postConstruct
+    )
+  }
+}
+
+/** An annotation type annotated with the `InjectedParameter` annotation. */
+private class InjectedParameterAnnotatedType extends AnnotationType {
+  InjectedParameterAnnotatedType() {
+    this.getAnAnnotation().getType().hasQualifiedName("org.kohsuke.stapler", "InjectedParameter")
+  }
+}
+
+/** The `generateResponse` method of `org.kohsuke.stapler.HttpResponse` or its subtypes. */
+private class GenerateResponseMethod extends Method {
+  GenerateResponseMethod() {
+    this.getDeclaringType().getASourceSupertype*() instanceof HttpResponse and
+    this.hasName("generateResponse")
+  }
+}
+
+/** Holds if `t` is the static type of `e`, or an upper bound of the runtime type of `e`. */
+private predicate boundOrStaticType(Expr e, RefType t) {
+  exprTypeFlow(e, t, false)
+  or
+  t = e.getType()
+}
+
+/**
+ * A method called after the construction of a `DataBound` object.
+ *
+ * That is, either the `bindResolve` method of a subtype of `org.kohsuke.stapler.DataBoundResolvable`,
+ * or a method annotated with `javax.annotation.PostConstruct`.
+ */
+private class PostConstructDataBoundMethod extends Method {
+  PostConstructDataBoundMethod() {
+    this.getDeclaringType()
+        .getASourceSupertype*()
+        .hasQualifiedName("org.kohsuke.stapler", "DataBoundResolvable") and
+    this.hasName("bindResolve")
+    or
+    this.getAnAnnotation() instanceof PostConstructAnnotation
+  }
+}

java/ql/test/library-tests/dataflow/taintsources/Stapler.java

@@ -0,0 +1,14 @@
+import org.kohsuke.stapler.InjectedParameter;
+
+public class Stapler {
+
+    @InjectedParameter
+    private @interface MyInjectedParameter {
+    }
+
+    private static void sink(Object o) {}
+
+    public static void test(@MyInjectedParameter String src) {
+        sink(src); // $ hasRemoteValueFlow
+    }
+}

java/ql/test/library-tests/dataflow/taintsources/options

@@ -1 +1 @@
-//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/servlet-api-2.4:${testdir}/../../../stubs/springframework-5.3.8:${testdir}/../../../stubs/google-android-9.0.0:${testdir}/../../../stubs/playframework-2.6.x:${testdir}/../../../stubs/jackson-databind-2.12:${testdir}/../../../stubs/jackson-core-2.12:${testdir}/../../../stubs/akka-2.6.x:${testdir}/../../../stubs/jwtk-jjwt-0.11.2:${testdir}/../../../stubs/jenkins
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/servlet-api-2.4:${testdir}/../../../stubs/springframework-5.3.8:${testdir}/../../../stubs/google-android-9.0.0:${testdir}/../../../stubs/playframework-2.6.x:${testdir}/../../../stubs/jackson-databind-2.12:${testdir}/../../../stubs/jackson-core-2.12:${testdir}/../../../stubs/akka-2.6.x:${testdir}/../../../stubs/jwtk-jjwt-0.11.2:${testdir}/../../../stubs/jenkins:${testdir}/../../../stubs/stapler-1.263

java/ql/test/library-tests/frameworks/stapler/DataBoundPostConstructTest.java

@@ -0,0 +1,42 @@
+import javax.annotation.PostConstruct;
+import net.sf.json.JSONObject;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.DataBoundResolvable;
+import org.kohsuke.stapler.DataBoundSetter;
+import org.kohsuke.stapler.StaplerRequest;
+
+public class DataBoundPostConstructTest implements DataBoundResolvable {
+
+    static Object source(String label) {
+        return null;
+    }
+
+    static void sink(Object o) {}
+
+    static void test() {
+        new DataBoundPostConstructTest(source("constructor"));
+        new DataBoundPostConstructTest(null).setField(source("setter"));
+    }
+
+    private Object field;
+
+    @DataBoundConstructor
+    public DataBoundPostConstructTest(Object field) {
+        this.field = field;
+    }
+
+    @DataBoundSetter
+    public void setField(Object field) {
+        this.field = field;
+    }
+
+    private Object bindResolve(StaplerRequest request, JSONObject src) {
+        sink(this.field); // $ hasValueFlow=constructor hasValueFlow=setter
+        return null;
+    }
+
+    @PostConstruct
+    private void post() {
+        sink(this.field); // $ hasValueFlow=constructor hasValueFlow=setter
+    }
+}

java/ql/test/library-tests/frameworks/stapler/HttpResponseTest.java

@@ -0,0 +1,26 @@
+import hudson.model.Descriptor;
+import org.kohsuke.stapler.HttpResponse;
+import org.kohsuke.stapler.StaplerRequest;
+import org.kohsuke.stapler.StaplerResponse;
+
+public class HttpResponseTest {
+
+    Object source() {
+        return null;
+    }
+
+    void sink(Object o) {}
+
+    private class MyDescriptor extends Descriptor<Object> {
+        public HttpResponse doTest() {
+            return (MyHttpResponse) source();
+        }
+    }
+
+    private class MyHttpResponse implements HttpResponse {
+        @Override
+        public void generateResponse(StaplerRequest p0, StaplerResponse p1, Object p2) {
+            sink(this); // $ hasValueFlow
+        }
+    }
+}

java/ql/test/library-tests/frameworks/stapler/options

@@ -1 +1 @@
-//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/stapler-1.263:${testdir}/../../../stubs/javax-servlet-2.5:${testdir}/../../../stubs/apache-commons-jelly-1.0.1:${testdir}/../../../stubs/apache-commons-fileupload-1.4:${testdir}/../../../stubs/saxon-xqj-9.x:${testdir}/../../../stubs/apache-commons-beanutils:${testdir}/../../../stubs/dom4j-2.1.1:${testdir}/../../../stubs/apache-commons-lang:${testdir}/../../../stubs/jaxen-1.2.0
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/stapler-1.263:${testdir}/../../../stubs/javax-servlet-2.5:${testdir}/../../../stubs/apache-commons-jelly-1.0.1:${testdir}/../../../stubs/apache-commons-fileupload-1.4:${testdir}/../../../stubs/saxon-xqj-9.x:${testdir}/../../../stubs/apache-commons-beanutils:${testdir}/../../../stubs/dom4j-2.1.1:${testdir}/../../../stubs/apache-commons-lang:${testdir}/../../../stubs/jaxen-1.2.0:${testdir}/../../../stubs/jenkins:${testdir}/../../../stubs/javax-annotation-api-1.3.2

java/ql/test/stubs/javax-annotation-api-1.3.2/javax/annotation/PostConstruct.java

@@ -0,0 +1,13 @@
+package javax.annotation;
+
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+import java.lang.annotation.Documented;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+@Documented
+@Retention(RUNTIME)
+@Target(METHOD)
+public @interface PostConstruct {
+}

java/ql/test/stubs/jenkins/hudson/model/Descriptor.java

@@ -0,0 +1,5 @@
+package hudson.model;
+
+public abstract class Descriptor<T> {
+
+}

java/ql/test/stubs/stapler-1.263/org/kohsuke/stapler/DataBoundConstructor.java

@@ -0,0 +1,13 @@
+package org.kohsuke.stapler;
+
+import static java.lang.annotation.ElementType.CONSTRUCTOR;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+import java.lang.annotation.Documented;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+@Retention(RUNTIME)
+@Target(CONSTRUCTOR)
+@Documented
+public @interface DataBoundConstructor {
+}

java/ql/test/stubs/stapler-1.263/org/kohsuke/stapler/DataBoundResolvable.java

@@ -0,0 +1,4 @@
+package org.kohsuke.stapler;
+
+public interface DataBoundResolvable {
+}

java/ql/test/stubs/stapler-1.263/org/kohsuke/stapler/DataBoundSetter.java

@@ -0,0 +1,14 @@
+package org.kohsuke.stapler;
+
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+import java.lang.annotation.Documented;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+@Retention(RUNTIME)
+@Target({METHOD, FIELD})
+@Documented
+public @interface DataBoundSetter {
+}

java/ql/test/stubs/stapler-1.263/org/kohsuke/stapler/InjectedParameter.java

@@ -0,0 +1,13 @@
+package org.kohsuke.stapler;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+import static java.lang.annotation.ElementType.ANNOTATION_TYPE;;
+
+@Retention(RUNTIME)
+@Target(ANNOTATION_TYPE)
+@Documented
+public @interface InjectedParameter {
+}