Merge pull request #461 from owen-mc/avoid-unused-barrier-guards-in-scope

Move reused barrier guards into separate files

ql/src/semmle/go/dataflow/BarrierGuardUtil.qll

@@ -1,68 +0,0 @@
-/**
- * Provides implementations of some commonly used barrier guards for sanitizing untrusted URLs.
- */
-
-import go
-
-/**
- * A call to a function called `isLocalUrl`, `isValidRedirect`, or similar, which is
- * considered a barrier guard for sanitizing untrusted URLs.
- */
-class RedirectCheckBarrierGuard extends DataFlow::BarrierGuard, DataFlow::CallNode {
-  RedirectCheckBarrierGuard() {
-    this.getCalleeName().regexpMatch("(?i)(is_?)?(local_?url|valid_?redir(ect)?)(ur[li])?")
-  }
-
-  override predicate checks(Expr e, boolean outcome) {
-    // `isLocalUrl(e)` is a barrier for `e` if it evaluates to `true`
-    getAnArgument().asExpr() = e and
-    outcome = true
-  }
-}
-
-/**
- * An equality check comparing a data-flow node against a constant string, considered as
- * a barrier guard for sanitizing untrusted URLs.
- *
- * Additionally, a check comparing `url.Hostname()` against a constant string is also
- * considered a barrier guard for `url`.
- */
-class UrlCheck extends DataFlow::BarrierGuard, DataFlow::EqualityTestNode {
-  DataFlow::Node url;
-
-  UrlCheck() {
-    exists(this.getAnOperand().getStringValue()) and
-    (
-      url = this.getAnOperand()
-      or
-      exists(DataFlow::MethodCallNode mc | mc = this.getAnOperand() |
-        mc.getTarget().getName() = "Hostname" and
-        url = mc.getReceiver()
-      )
-    )
-  }
-
-  override predicate checks(Expr e, boolean outcome) {
-    e = url.asExpr() and outcome = this.getPolarity()
-  }
-}
-
-/**
- * A call to a regexp match function, considered as a barrier guard for sanitizing untrusted URLs.
- *
- * This is overapproximate: we do not attempt to reason about the correctness of the regexp.
- */
-class RegexpCheck extends DataFlow::BarrierGuard {
-  RegexpMatchFunction matchfn;
-  DataFlow::CallNode call;
-
-  RegexpCheck() {
-    matchfn.getACall() = call and
-    this = matchfn.getResult().getNode(call).getASuccessor*()
-  }
-
-  override predicate checks(Expr e, boolean branch) {
-    e = matchfn.getValue().getNode(call).asExpr() and
-    (branch = false or branch = true)
-  }
-}

ql/src/semmle/go/dataflow/barrierguardutil/RedirectCheckBarrierGuard.qll

@@ -0,0 +1,21 @@
+/**
+ * Provides an implementation of a commonly used barrier guard for sanitizing untrusted URLs.
+ */
+
+import go
+
+/**
+ * A call to a function called `isLocalUrl`, `isValidRedirect`, or similar, which is
+ * considered a barrier guard for sanitizing untrusted URLs.
+ */
+class RedirectCheckBarrierGuard extends DataFlow::BarrierGuard, DataFlow::CallNode {
+  RedirectCheckBarrierGuard() {
+    this.getCalleeName().regexpMatch("(?i)(is_?)?(local_?url|valid_?redir(ect)?)(ur[li])?")
+  }
+
+  override predicate checks(Expr e, boolean outcome) {
+    // `isLocalUrl(e)` is a barrier for `e` if it evaluates to `true`
+    getAnArgument().asExpr() = e and
+    outcome = true
+  }
+}

ql/src/semmle/go/dataflow/barrierguardutil/RegexpCheck.qll

@@ -0,0 +1,25 @@
+/**
+ * Provides an implementation of a commonly used barrier guard for sanitizing untrusted URLs.
+ */
+
+import go
+
+/**
+ * A call to a regexp match function, considered as a barrier guard for sanitizing untrusted URLs.
+ *
+ * This is overapproximate: we do not attempt to reason about the correctness of the regexp.
+ */
+class RegexpCheck extends DataFlow::BarrierGuard {
+  RegexpMatchFunction matchfn;
+  DataFlow::CallNode call;
+
+  RegexpCheck() {
+    matchfn.getACall() = call and
+    this = matchfn.getResult().getNode(call).getASuccessor*()
+  }
+
+  override predicate checks(Expr e, boolean branch) {
+    e = matchfn.getValue().getNode(call).asExpr() and
+    (branch = false or branch = true)
+  }
+}

ql/src/semmle/go/dataflow/barrierguardutil/UrlCheck.qll

@@ -0,0 +1,32 @@
+/**
+ * Provides an implementation of a commonly used barrier guard for sanitizing untrusted URLs.
+ */
+
+import go
+
+/**
+ * An equality check comparing a data-flow node against a constant string, considered as
+ * a barrier guard for sanitizing untrusted URLs.
+ *
+ * Additionally, a check comparing `url.Hostname()` against a constant string is also
+ * considered a barrier guard for `url`.
+ */
+class UrlCheck extends DataFlow::BarrierGuard, DataFlow::EqualityTestNode {
+  DataFlow::Node url;
+
+  UrlCheck() {
+    exists(this.getAnOperand().getStringValue()) and
+    (
+      url = this.getAnOperand()
+      or
+      exists(DataFlow::MethodCallNode mc | mc = this.getAnOperand() |
+        mc.getTarget().getName() = "Hostname" and
+        url = mc.getReceiver()
+      )
+    )
+  }
+
+  override predicate checks(Expr e, boolean outcome) {
+    e = url.asExpr() and outcome = this.getPolarity()
+  }
+}

ql/src/semmle/go/security/OpenUrlRedirectCustomizations.qll

@@ -7,7 +7,9 @@
 import go
 import UrlConcatenation
 import SafeUrlFlowCustomizations
-import semmle.go.dataflow.BarrierGuardUtil
+import semmle.go.dataflow.barrierguardutil.RedirectCheckBarrierGuard
+import semmle.go.dataflow.barrierguardutil.RegexpCheck
+import semmle.go.dataflow.barrierguardutil.UrlCheck
 
 /**
  * Provides extension points for customizing the taint-tracking configuration for reasoning about

ql/src/semmle/go/security/RequestForgeryCustomizations.qll

@@ -5,7 +5,9 @@
 import go
 import UrlConcatenation
 import SafeUrlFlowCustomizations
-import semmle.go.dataflow.BarrierGuardUtil
+import semmle.go.dataflow.barrierguardutil.RedirectCheckBarrierGuard
+import semmle.go.dataflow.barrierguardutil.RegexpCheck
+import semmle.go.dataflow.barrierguardutil.UrlCheck
 
 /** Provides classes and predicates for the request forgery query. */
 module RequestForgery {

ql/src/semmle/go/security/TaintedPathCustomizations.qll

@@ -4,7 +4,7 @@
  */
 
 import go
-import semmle.go.dataflow.BarrierGuardUtil
+import semmle.go.dataflow.barrierguardutil.RegexpCheck
 
 /**
  * Provides extension points for customizing the taint tracking configuration for reasoning about