feat(rules): Tune XSS rules, separate ERROR and NOTE version

.github/workflows/ci-analyzer-owasp.yaml

@@ -21,7 +21,7 @@ concurrency:
   cancel-in-progress: true
 
 env:
-  EXPECTED_TRACES: 3011
+  EXPECTED_TRACES: 3835
 
 jobs:
   owasp:

core/opentaint-java-querylang/samples/src/main/java/issues/i98/Builder.java

@@ -0,0 +1,9 @@
+package issues.i98;
+
+public class Builder {
+    public void cfg(String key, String value) { }
+
+    public void sink(String taint) { }
+
+    public static String source() { return "oops"; }
+}

core/opentaint-java-querylang/samples/src/main/java/issues/issue98.java

@@ -0,0 +1,64 @@
+package issues;
+
+import base.RuleSample;
+import base.RuleSet;
+import issues.i98.Builder;
+
+/**
+ * Engine bug: a `pattern-not-inside` whose discriminator-arg metavariable is
+ * narrowed by a sibling `metavariable-pattern` is silently ignored.
+ *
+ * The rule (issue98.yaml):
+ *   - binds `$X` via `pattern-inside: Builder $X = new Builder(); ...`
+ *   - binds `$UNTRUSTED` via the positive sink `$X.sink($UNTRUSTED)`
+ *   - subtracts via `pattern-not-inside: $X.cfg("Content-Type", $V)` plus
+ *     `metavariable-pattern: $V matches "application/json"`
+ *
+ * Every metavariable in the pattern-not-inside is bound: `$X` from
+ * `pattern-inside`, `$V` constrained by `metavariable-pattern`. With the
+ * literal-arg form `pattern-not-inside: $X.cfg("Content-Type",
+ * "application/json")` (no metavariable-pattern) the engine correctly
+ * subtracts the match — this test passes. Replacing the literal arg with a
+ * metavariable + `metavariable-pattern` constraint (the same shape the XSS
+ * rule uses for `$CT_SAFE`) loses the subtraction.
+ *
+ * Expected:
+ *   - PositiveStmtNoCfg fires (no `.cfg` call in scope).
+ *   - NegativeStmtWithCfg is subtracted (`.cfg("Content-Type",
+ *     "application/json")` matches the pattern-not-inside, and
+ *     `metavariable-pattern` accepts the literal).
+ *
+ * Actual: NegativeStmtWithCfg also fires.
+ *
+ * Motivation: matches the `(HttpServletResponse $R).setContentType($CT_SAFE); …`
+ * + `metavariable-pattern: $CT_SAFE ∈ {non-HTML strings}` subtractor in
+ * `servlet-xss-html-response-sinks.yaml` that fails to clear the
+ * `SafeChainedWriterJsonServlet` / `SafeOutputStreamOctetServlet` FPs.
+ */
+@RuleSet("issues/issue98.yaml")
+public abstract class issue98 implements RuleSample {
+    /** No discriminator call in scope — sink should fire. */
+    static class PositiveStmtNoCfg extends issue98 {
+        @Override
+        public void entrypoint() {
+            Builder b = new Builder();
+            b.sink(Builder.source());
+        }
+    }
+
+    /**
+     * Discriminator `b.cfg("Content-Type", "application/json")` is present in
+     * the method body. The pattern-not-inside + metavariable-pattern pair
+     * should subtract this match — but does not. Disabled until the engine
+     * honors metavariable-pattern-narrowed arguments inside
+     * pattern-not-inside.
+     */
+    static class NegativeStmtWithCfg extends issue98 {
+        @Override
+        public void entrypoint() {
+            Builder b = new Builder();
+            b.cfg("Content-Type", "application/json");
+            b.sink(Builder.source());
+        }
+    }
+}

core/opentaint-java-querylang/samples/src/main/resources/issues/issue98.yaml

@@ -0,0 +1,30 @@
+rules:
+  - id: i98
+    message: >
+      pattern-not-inside whose discriminator-arg metavariable is narrowed by
+      a sibling metavariable-pattern is silently ignored. Replacing the
+      metavariable with the equivalent literal makes the subtraction work.
+      See issue98.java for the demonstration and the motivating XSS rule
+      context (`servlet-xss-html-response-sinks.yaml` subtractor on
+      `(HttpServletResponse $R).setContentType($CT_SAFE); ...` plus
+      `metavariable-pattern: $CT_SAFE ∈ {non-HTML strings}`).
+    severity: ERROR
+    languages:
+      - java
+    mode: taint
+    pattern-sources:
+      - pattern: source()
+    pattern-sinks:
+      - patterns:
+          - pattern-inside: |
+              $X = new Builder();
+              ...
+          - patterns:
+              - pattern-not-inside: $X.cfg("Content-Type", $V)
+              - metavariable-pattern:
+                  metavariable: $V
+                  patterns:
+                    - pattern-either:
+                        - pattern: '"application/json"'
+          - pattern: $X.sink($UNTRUSTED)
+          - focus-metavariable: $UNTRUSTED

core/opentaint-java-querylang/src/test/kotlin/org/opentaint/semgrep/IssuesTest.kt

@@ -16,6 +16,7 @@ import issues.issue94
 import issues.issue95
 import issues.issue96
 import issues.issue97
+import issues.issue98
 import org.junit.jupiter.api.AfterAll
 import org.junit.jupiter.api.Disabled
 import org.junit.jupiter.api.TestInstance
@@ -105,6 +106,10 @@ class IssuesTest : SampleBasedTest() {
     @Test
     fun `issue 97`() = runTest<issue97>()
 
+    @Test
+    @Disabled // todo: pattern-not-inside ignored when its discriminator metavariable is narrowed by a sibling metavariable-pattern
+    fun `issue 98`() = runTest<issue98>()
+
     @AfterAll
     fun close() {
         closeRunner()

rules/ruleset/java/lib/generic/servlet-xss-html-response-sinks.yaml

@@ -0,0 +1,108 @@
+rules:
+  - id: java-servlet-xss-html-response-sink
+    options:
+      lib: true
+    severity: NOTE
+    message: Direct write of unvalidated user input into a response without safe content type
+    metadata:
+      provenance:
+        - https://github.com/github/codeql/blob/main/java/ql/lib/semmle/code/java/security/XSS.qll
+    languages:
+      - java
+    mode: taint
+
+    # Servlet defaults Content-Type to text/html — writer/OutputStream is XSS
+    # unless setContentType pins non-HTML. Spring sibling for raw HttpServlet handlers.
+
+    pattern-sanitizers:
+      - patterns:
+          - pattern-either:
+              - pattern: Encode.forHtml(..., $UNTRUSTED, ...)
+              - pattern: (PolicyFactory $POLICY).sanitize(..., $UNTRUSTED, ...)
+              - pattern: (AntiSamy $AS).scan(..., $UNTRUSTED, ...)
+              - pattern: JSoup.clean(..., $UNTRUSTED, ...)
+              - pattern: HtmlUtils.htmlEscape(..., $UNTRUSTED, ...)
+              - pattern: org.apache.commons.lang.StringEscapeUtils.escapeHtml(..., $UNTRUSTED, ...)
+              - pattern: org.apache.commons.text.StringEscapeUtils.escapeHtml3(..., $UNTRUSTED, ...)
+              - pattern: org.apache.commons.text.StringEscapeUtils.escapeHtml4(..., $UNTRUSTED, ...)
+              - pattern: org.owasp.esapi.ESAPI.encoder().encodeForHTML(..., $UNTRUSTED, ...)
+          - focus-metavariable: $UNTRUSTED
+
+    pattern-sinks:
+      - patterns:
+          - patterns:
+            - pattern-either:
+              - pattern-inside: |
+                  $RETURNTYPE $ENTRYPOINT(..., javax.servlet.http.HttpServletResponse $RESPONSE, ...) {
+                  ...
+                  }
+            - pattern-inside: |
+                  $RETURNTYPE $ENTRYPOINT(..., jakarta.servlet.http.HttpServletResponse $RESPONSE, ...) {
+                  ...
+                  }
+            - metavariable-pattern:
+                metavariable: $ENTRYPOINT
+                pattern-either:
+                  - pattern: doDelete
+                  - pattern: doGet
+                  - pattern: doPost
+                  - pattern: doPut
+                  - pattern: doTrace
+                  - pattern: _jspService
+          - pattern-either:
+              - patterns:
+                - pattern-either:
+                  - pattern-inside: |
+                      $W = (javax.servlet.http.HttpServletResponse $RESPONSE).getWriter(...);
+                      ...
+                  - pattern-inside: |
+                      $W = (jakarta.servlet.http.HttpServletResponse $RESPONSE).getWriter(...);
+                      ...
+                - pattern: |
+                    $W.$WRITE(..., $UNTRUSTED, ...);
+              - patterns:
+                - pattern-either:
+                  - pattern-inside: |
+                      $S = (javax.servlet.http.HttpServletResponse $RESPONSE).getOutputStream(...);
+                      ...
+                  - pattern-inside: |
+                      $S = (jakarta.servlet.http.HttpServletResponse $RESPONSE).getOutputStream(...);
+                      ...
+                - pattern: |
+                    $S.$WRITE(..., $UNTRUSTED, ...);
+              - pattern: (HttpServletResponse $RESPONSE).sendError($CODE, $UNTRUSTED)
+              - pattern: (jakarta.servlet.jsp.JspWriter $W).$WRITE(..., $UNTRUSTED, ...)
+              - pattern: (javax.servlet.jsp.JspWriter $W).$WRITE(..., $UNTRUSTED, ...)
+
+          # $CT_SAFE OUTSIDE quotes binds to the whole string-literal expression;
+          # bare `application/json` can't be used — engine parses `/` `-` as Java operators.
+          - patterns:
+              - pattern-not-inside: |
+                  (HttpServletResponse $RESPONSE).setContentType($CT_SAFE);
+                  ...
+              - metavariable-pattern:
+                  metavariable: $CT_SAFE
+                  patterns:
+                    - pattern-either:
+                        - pattern: '"application/json"'
+                        - pattern: '"application/json;charset=UTF-8"'
+                        - pattern: '"text/plain"'
+                        - pattern: '"application/pdf"'
+                        - pattern: '"application/octet-stream"'
+                        - pattern: '"application/xml"'
+                        - pattern: '"text/xml"'
+                        - pattern: '"image/png"'
+                        - pattern: '"image/jpeg"'
+                        - pattern: '"image/gif"'
+                        # MediaType.X_VALUE constants — dev analyzer constant-folds; kept
+                        # explicit for analyzer-regression / older-build robustness.
+                        - pattern: MediaType.APPLICATION_JSON_VALUE
+                        - pattern: MediaType.TEXT_PLAIN_VALUE
+                        - pattern: MediaType.APPLICATION_PDF_VALUE
+                        - pattern: MediaType.APPLICATION_OCTET_STREAM_VALUE
+                        - pattern: MediaType.APPLICATION_XML_VALUE
+                        - pattern: MediaType.TEXT_XML_VALUE
+                        - pattern: MediaType.IMAGE_PNG_VALUE
+                        - pattern: MediaType.IMAGE_JPEG_VALUE
+                        - pattern: MediaType.IMAGE_GIF_VALUE
+          - focus-metavariable: $UNTRUSTED

rules/ruleset/java/lib/generic/servlet-xss-sinks.yaml

@@ -10,34 +10,63 @@ rules:
       - java
     mode: taint
     pattern-sanitizers:
-      - pattern-either:
-          - pattern: Encode.forHtml(...)
-          - pattern: (PolicyFactory $POLICY).sanitize(...)
-          - pattern: (AntiSamy $AS).scan(...)
-          - pattern: JSoup.clean(...)
-          - pattern: HtmlUtils.htmlEscape(...)
-          - pattern: org.apache.commons.lang.StringEscapeUtils.escapeHtml(...)
-          - pattern: org.apache.commons.text.StringEscapeUtils.escapeHtml3(...)
-          - pattern: org.apache.commons.text.StringEscapeUtils.escapeHtml4(...)
-          - pattern: org.owasp.esapi.ESAPI.encoder().encodeForHTML(...)
+      - patterns:
+          - pattern-either:
+              - pattern: Encode.forHtml(..., $UNTRUSTED, ...)
+              - pattern: (PolicyFactory $POLICY).sanitize(..., $UNTRUSTED, ...)
+              - pattern: (AntiSamy $AS).scan(..., $UNTRUSTED, ...)
+              - pattern: JSoup.clean(..., $UNTRUSTED, ...)
+              - pattern: HtmlUtils.htmlEscape(..., $UNTRUSTED, ...)
+              - pattern: org.apache.commons.lang.StringEscapeUtils.escapeHtml(..., $UNTRUSTED, ...)
+              - pattern: org.apache.commons.text.StringEscapeUtils.escapeHtml3(..., $UNTRUSTED, ...)
+              - pattern: org.apache.commons.text.StringEscapeUtils.escapeHtml4(..., $UNTRUSTED, ...)
+              - pattern: org.owasp.esapi.ESAPI.encoder().encodeForHTML(..., $UNTRUSTED, ...)
+          - focus-metavariable: $UNTRUSTED
 
     pattern-sinks:
       - patterns:
           - pattern-either:
-              - pattern: |
-                  (HttpServletResponse $RESPONSE).getWriter(...).$WRITE(..., $UNTRUSTED, ...)
-              - pattern: |
-                  (HttpServletResponse $RESPONSE).getOutputStream(...).$WRITE(..., $UNTRUSTED, ...)
-              - pattern: |
-                  (HttpServletResponse $RESPONSE).sendError($CODE, $UNTRUSTED)
-              - pattern: |
-                  (java.io.PrintWriter $WRITER).$WRITE(..., $UNTRUSTED, ...)
-              - pattern: |
-                  (PrintWriter $WRITER).$WRITE(..., $UNTRUSTED, ...)
-              - pattern: |
-                  (javax.servlet.ServletOutputStream $WRITER).$WRITE(..., $UNTRUSTED, ...)
-              - pattern: |
-                  (ServletOutputStream $WRITER).$WRITE(..., $UNTRUSTED, ...)
-              - pattern: |
-                  (jakarta.servlet.jsp.JspWriter $WRITER).$WRITE(..., $UNTRUSTED, ...)
+              - patterns:
+                - patterns:
+                  - pattern-either:
+                    - pattern-inside: |
+                        $RETURNTYPE $ENTRYPOINT(..., javax.servlet.http.HttpServletResponse $RESPONSE, ...) {
+                        ...
+                        }
+                    - pattern-inside: |
+                        $RETURNTYPE $ENTRYPOINT(..., jakarta.servlet.http.HttpServletResponse $RESPONSE, ...) {
+                        ...
+                        }
+                  - metavariable-pattern:
+                      metavariable: $ENTRYPOINT
+                      pattern-either:
+                        - pattern: doDelete
+                        - pattern: doGet
+                        - pattern: doPost
+                        - pattern: doPut
+                        - pattern: doTrace
+                        - pattern: _jspService
+                - patterns:
+                  - pattern-either:
+                    - pattern-inside: |
+                        $W = (javax.servlet.http.HttpServletResponse $RESPONSE).getWriter(...);
+                        ...
+                    - pattern-inside: |
+                        $W = (jakarta.servlet.http.HttpServletResponse $RESPONSE).getWriter(...);
+                        ...
+                  - pattern: |
+                      $W.$WRITE(..., $UNTRUSTED, ...);
+                - patterns:
+                  - pattern-either:
+                    - pattern-inside: |
+                        $S = (javax.servlet.http.HttpServletResponse $RESPONSE).getOutputStream(...);
+                        ...
+                    - pattern-inside: |
+                        $S = (jakarta.servlet.http.HttpServletResponse $RESPONSE).getOutputStream(...);
+                        ...
+                  - pattern: |
+                      $S.$WRITE(..., $UNTRUSTED, ...);
+              - pattern: (HttpServletResponse $RESPONSE).sendError($CODE, $UNTRUSTED)
+              - pattern: (jakarta.servlet.jsp.JspWriter $W).$WRITE(..., $UNTRUSTED, ...)
+              - pattern: (javax.servlet.jsp.JspWriter $W).$WRITE(..., $UNTRUSTED, ...)
           - focus-metavariable: $UNTRUSTED