From 260b33be7ec92d7eedfb6388b25de8e2be8ee1e7 Mon Sep 17 00:00:00 2001
From: Sauyon Lee <sauyon@github.com>
Date: Thu, 9 Jan 2020 04:28:59 -0800
Subject: [PATCH] OpenUrlRedirect: Add untrusted methods

Also use more up-to-date data-flow APIs
---
 .../OpenUrlRedirectCustomizations.qll         | 36 +++++++++++++++----
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/ql/src/semmle/go/security/OpenUrlRedirectCustomizations.qll b/ql/src/semmle/go/security/OpenUrlRedirectCustomizations.qll
index 6285fbec60e..f496a4abe46 100644
--- a/ql/src/semmle/go/security/OpenUrlRedirectCustomizations.qll
+++ b/ql/src/semmle/go/security/OpenUrlRedirectCustomizations.qll
@@ -42,16 +42,16 @@ module OpenUrlRedirect {
   /**
    * An access to a user-controlled URL field, considered as a flow source for URL redirects.
    */
-  class UntrustedUrlField extends Source, DataFlow::ExprNode {
-    override SelectorExpr expr;
-
+  class UntrustedUrlField extends Source, DataFlow::FieldReadNode {
     UntrustedUrlField() {
-      exists(Type req, Type baseType, string fieldName |
+      exists(Type req, Type baseType, string fieldName, DataFlow::FieldReadNode url |
         req.hasQualifiedName("net/http", "Request") and
-        baseType = expr.getBase().(SelectorExpr).getBase().getType() and
-        expr.getBase().(SelectorExpr).getSelector().getName() = "URL" and
+        baseType = url.getBase().getType() and
         (baseType = req or baseType = req.getPointerType()) and
-        fieldName = expr.getSelector().getName() and
+        url.getFieldName() = "URL"
+      |
+        this.getBase() = url.getASuccessor*() and
+        this.getFieldName() = fieldName and
         (
           fieldName = "User" or
           fieldName = "Path" or
@@ -64,6 +64,28 @@ module OpenUrlRedirect {
     }
   }
 
+  /**
+   * An call to a user-controlled URL method, considered as a flow source for URL redirects.
+   */
+  class UntrustedUrlMethod extends Source, DataFlow::MethodCallNode {
+    UntrustedUrlMethod() {
+      exists(Type req, Type baseType, string methodName, DataFlow::FieldReadNode url |
+        req.hasQualifiedName("net/http", "Request") and
+        baseType = url.getBase().getType() and
+        (baseType = req or baseType = req.getPointerType()) and
+        url.getFieldName() = "URL"
+      |
+        this.getReceiver() = url.getASuccessor*() and
+        this.getCalleeName() = methodName and
+        (
+          methodName = "EscapedPath" or
+          methodName = "Query" or
+          methodName = "RequestURI"
+        )
+      )
+    }
+  }
+
   /**
    * An HTTP redirect, considered as a sink for `Configuration`.
    */