Merge pull request #209 from sauyon/bad-redirect-sanitiser

Bad redirect sanitiser

change-notes/1.24/analysis-go.md

@@ -10,6 +10,7 @@
 
 | **Query**                                                                 | **Tags**                                                                   | **Purpose**                                                                                                                                            |
 |---------------------------------------------------------------------------|----------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------|
+| Bad check of redirect URL (`go/bad-redirect-check`) | correctness, security, external/cwe/cwe-601 | Highlights checks that ensure redirect URLs start with `/` but don't check for `//` or `/\`. Results are shown on LGTM by default. |
 | Constant length comparison (`go/constant-length-comparison`)     | correctness | Highlights code that checks the length of an array or slice against a constant before indexing it using a variable, suggesting a logic error. Results are shown on LGTM by default. |
 | Impossible interface nil check (`go/impossible-interface-nil-check`) | correctness | Highlights code that compares an interface value that cannot be `nil` to `nil`, suggesting a logic error. Results are shown on LGTM by default. |
 | Incomplete URL scheme check (`go/incomplete-url-scheme-check`) | correctness, security, external/cwe/cwe-020 | Highlights checks for `javascript` URLs that do not take `data` or `vbscript` URLs into account. Results are shown on LGTM by default. |

ql/src/Security/CWE-601/BadRedirectCheck.go

@@ -0,0 +1,8 @@
+package main
+
+func sanitizeUrl(redir string) string {
+	if len(redir) > 0 && redir[0] == '/' {
+		return redir
+	}
+	return "/"
+}

ql/src/Security/CWE-601/BadRedirectCheck.qhelp

@@ -0,0 +1,40 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Redirect URLs should be checked to ensure that user input cannot cause a site to redirect
+to arbitrary domains. This is often done with a check that the redirect URL begins with a slash,
+which most of the time is an absolute redirect on the same host. However, browsers interpret URLs
+beginning with <code>//</code> or <code>/\</code> as absolute URLs. For example, a redirect to
+<code>//lgtm.com</code> will redirect to <code>https://lgtm.com</code>. Thus, redirect checks must
+also check the second character of redirect URLs.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Also disallow redirect URLs starting with <code>//</code> or <code>/\</code>.
+</p>
+</recommendation>
+
+<example>
+<p>
+The following function validates a (presumably untrusted) redirect URL <code>redir</code>. If it
+does not begin with <code>/</code>, the harmless placeholder redirect URL <code>/</code> is
+returned to prevent an open redirect; otherwise <code>redir</code> itself is returned.
+</p>
+<sample src="BadRedirectCheck.go"/>
+<p>
+While this check provides partial protection, it should be extended to cover <code>//</code> and
+<code>/\</code> as well:
+</p>
+<sample src="BadRedirectCheckGood.go"/>
+</example>
+
+<references>
+<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html#validating-urls">
+  XSS Unvalidated Redirects and Forwards Cheat Sheet</a>.</li>
+</references>
+
+</qhelp>

ql/src/Security/CWE-601/BadRedirectCheck.ql

@@ -0,0 +1,65 @@
+/**
+ * @name Bad redirect check
+ * @description A redirect check that checks for a leading slash but not two
+ *              leading slashes or a leading slash followed by a backslash is
+ *              incomplete.
+ * @kind problem
+ * @problem.severity warning
+ * @id go/bad-redirect-check
+ * @tags security
+ *       external/cwe/cwe-601
+ * @precision high
+ */
+
+import go
+
+StringOps::HasPrefix checkForLeadingSlash(SsaWithFields v) {
+  exists(DataFlow::Node substr |
+    result.getBaseString() = v.getAUse() and result.getSubstring() = substr
+  |
+    substr.getStringValue() = "/"
+    or
+    substr.getIntValue() = 47 // ASCII value for '/'
+  )
+}
+
+DataFlow::Node checkForSecondSlash(SsaWithFields v) {
+  exists(StringOps::HasPrefix hp | result = hp and hp.getBaseString() = v.getAUse() |
+    hp.getSubstring().getStringValue() = "//"
+  )
+  or
+  exists(DataFlow::EqualityTestNode eq, DataFlow::Node slash, DataFlow::ElementReadNode er |
+    result = eq
+  |
+    slash.getIntValue() = 47 and // ASCII value for '/'
+    er.getBase() = v.getAUse() and
+    er.getIndex().getIntValue() = 1 and
+    eq.eq(_, er, slash)
+  )
+}
+
+DataFlow::Node checkForSecondBackslash(SsaWithFields v) {
+  exists(StringOps::HasPrefix hp | result = hp and hp.getBaseString() = v.getAUse() |
+    hp.getSubstring().getStringValue() = "/\\"
+  )
+  or
+  exists(DataFlow::EqualityTestNode eq, DataFlow::Node slash, DataFlow::ElementReadNode er |
+    result = eq
+  |
+    slash.getIntValue() = 92 and // ASCII value for '\'
+    er.getBase() = v.getAUse() and
+    er.getIndex().getIntValue() = 1 and
+    eq.eq(_, er, slash)
+  )
+}
+
+from DataFlow::Node node, SsaWithFields v
+where
+  // there is a check for a leading slash
+  node = checkForLeadingSlash(v) and
+  // but not a check for both a second slash and a second backslash
+  not (exists(checkForSecondSlash(v)) and exists(checkForSecondBackslash(v))) and
+  v.getQualifiedName().regexpMatch("(?i).*url.*|.*redir.*|.*target.*")
+select node,
+  "This expression checks '$@' for a leading slash but checks do not exist for both '/' and '\\' in the second position.",
+  v, v.getQualifiedName()

ql/src/Security/CWE-601/BadRedirectCheckGood.go

@@ -0,0 +1,8 @@
+package main
+
+func sanitizeUrl1(redir string) string {
+	if len(redir) > 1 && redir[0] == '/' && redir[1] != '/' && redir[1] != '\\' {
+		return redir
+	}
+	return "/"
+}

ql/src/semmle/go/Expr.qll

@@ -318,7 +318,9 @@ class ParenExpr extends @parenexpr, Expr {
 
   override Expr stripParens() { result = getExpression().stripParens() }
 
-  override predicate isPlatformIndependentConstant() { getExpression().isPlatformIndependentConstant() }
+  override predicate isPlatformIndependentConstant() {
+    getExpression().isPlatformIndependentConstant()
+  }
 
   override string toString() { result = "(...)" }
 }
@@ -393,7 +395,9 @@ class TypeAssertExpr extends @typeassertexpr, Expr {
 
   override predicate mayHaveOwnSideEffects() { any() }
 
-  override predicate isPlatformIndependentConstant() { getExpression().isPlatformIndependentConstant() }
+  override predicate isPlatformIndependentConstant() {
+    getExpression().isPlatformIndependentConstant()
+  }
 
   override string toString() { result = "type assertion" }
 }
@@ -419,7 +423,9 @@ class ConversionExpr extends CallOrConversionExpr {
   /** Gets the operand of the type conversion. */
   Expr getOperand() { result = getChildExpr(1) }
 
-  override predicate isPlatformIndependentConstant() { getOperand().isPlatformIndependentConstant() }
+  override predicate isPlatformIndependentConstant() {
+    getOperand().isPlatformIndependentConstant()
+  }
 
   override string toString() { result = "type conversion" }
 }
@@ -638,6 +644,9 @@ class MapTypeExpr extends @maptypeexpr, Expr {
 class OperatorExpr extends @operatorexpr, Expr {
   /** Gets the operator of this expression. */
   string getOperator() { none() }
+
+  /** Gets an operand of this expression. */
+  Expr getAnOperand() { none() }
 }
 
 /**
@@ -662,7 +671,11 @@ class UnaryExpr extends @unaryexpr, OperatorExpr {
   /** Gets the operand of this unary expression. */
   Expr getOperand() { result = getChildExpr(0) }
 
-  override predicate isPlatformIndependentConstant() { getOperand().isPlatformIndependentConstant() }
+  override Expr getAnOperand() { result = this.getOperand() }
+
+  override predicate isPlatformIndependentConstant() {
+    getOperand().isPlatformIndependentConstant()
+  }
 
   override string toString() { result = getOperator() + "..." }
 }
@@ -747,8 +760,7 @@ class BinaryExpr extends @binaryexpr, OperatorExpr {
   /** Gets the right operand of this binary expression. */
   Expr getRightOperand() { result = getChildExpr(1) }
 
-  /** Gets an operand of this binary expression. */
-  Expr getAnOperand() { result = getChildExpr([0 .. 1]) }
+  override Expr getAnOperand() { result = getChildExpr([0 .. 1]) }
 
   /** Holds if `e` and `f` (in either order) are the two operands of this binary expression. */
   predicate hasOperands(Expr e, Expr f) {

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

@@ -10,9 +10,7 @@ private import SsaImpl
  * declared in file scope.
  */
 class SsaSourceVariable extends LocalVariable {
-  SsaSourceVariable() {
-    not getScope() instanceof FileScope
-  }
+  SsaSourceVariable() { not getScope() instanceof FileScope }
 
   /**
    * Holds if there may be indirect references of this variable that are not covered by `getAReference()`.
@@ -339,6 +337,20 @@ class SsaWithFields extends TSsaWithFields {
     exists(SsaWithFields base, Field f | this = TStep(base, f) | result = base + "." + f.getName())
   }
 
+  /**
+   * Gets the qualified name of the source variable or variable and fields that this represents.
+   *
+   * For example, for an SSA variable that represents the field `a.b`, this would get the string
+   * `"a.b"`.
+   */
+  string getQualifiedName() {
+    exists(SsaVariable v | this = TRoot(v) and result = v.getSourceVariable().getName())
+    or
+    exists(SsaWithFields base, Field f | this = TStep(base, f) |
+      result = base.getQualifiedName() + "." + f.getName()
+    )
+  }
+
   /**
    * Holds if this element is at the specified location.
    * The location spans column `startcolumn` of line `startline` to

ql/test/library-tests/semmle/go/dataflow/SSA/DefUse.expected

@@ -26,5 +26,9 @@
 | main.go:82:25:82:25 | implicit read of b | main.go:82:25:82:25 | definition of b | main.go:82:25:82:25 | b |
 | main.go:84:9:84:9 | x | main.go:83:2:83:2 | definition of x | main.go:83:2:83:2 | x |
 | main.go:84:15:84:15 | x | main.go:83:2:83:2 | definition of x | main.go:83:2:83:2 | x |
-| main.go:94:2:94:8 | wrapper | main.go:92:22:92:28 | definition of wrapper | main.go:92:22:92:28 | wrapper |
-| main.go:97:9:97:9 | x | main.go:94:2:96:3 | capture variable x | main.go:93:2:93:2 | x |
+| main.go:97:2:97:8 | wrapper | main.go:95:22:95:28 | definition of wrapper | main.go:95:22:95:28 | wrapper |
+| main.go:100:9:100:9 | x | main.go:97:2:99:3 | capture variable x | main.go:96:2:96:2 | x |
+| main.go:117:2:117:2 | p | main.go:117:2:117:2 | p = phi(def@112:3, def@114:3) | main.go:110:6:110:6 | p |
+| main.go:119:12:119:12 | p | main.go:117:2:117:2 | p = phi(def@112:3, def@114:3) | main.go:110:6:110:6 | p |
+| main.go:119:17:119:17 | p | main.go:117:2:117:2 | p = phi(def@112:3, def@114:3) | main.go:110:6:110:6 | p |
+| main.go:119:24:119:24 | p | main.go:117:2:117:2 | p = phi(def@112:3, def@114:3) | main.go:110:6:110:6 | p |

ql/test/library-tests/semmle/go/dataflow/SSA/SsaDefinition.expected

@@ -32,7 +32,10 @@
 | main.go:82:25:82:25 | definition of b |
 | main.go:83:2:83:2 | definition of x |
 | main.go:84:5:84:5 | definition of a |
-| main.go:92:22:92:28 | definition of wrapper |
-| main.go:93:2:93:2 | definition of x |
-| main.go:94:2:96:3 | capture variable x |
-| main.go:95:3:95:3 | definition of x |
+| main.go:95:22:95:28 | definition of wrapper |
+| main.go:96:2:96:2 | definition of x |
+| main.go:97:2:99:3 | capture variable x |
+| main.go:98:3:98:3 | definition of x |
+| main.go:112:3:112:3 | definition of p |
+| main.go:114:3:114:3 | definition of p |
+| main.go:117:2:117:2 | p = phi(def@112:3, def@114:3) |

ql/test/library-tests/semmle/go/dataflow/SSA/SsaWithFields.expected

@@ -0,0 +1,46 @@
+| main.go:13:6:13:6 | (def@13:6) | x |
+| main.go:14:2:14:2 | (def@14:2) | y |
+| main.go:17:3:17:3 | (def@17:3) | y |
+| main.go:19:2:19:2 | (phi@19:2) | y |
+| main.go:21:3:21:3 | (def@21:3) | x |
+| main.go:23:2:23:2 | (phi@23:2) | x |
+| main.go:26:10:26:10 | (def@26:10) | x |
+| main.go:27:2:27:2 | (def@27:2) | a |
+| main.go:27:5:27:5 | (def@27:5) | b |
+| main.go:29:3:29:3 | (def@29:3) | a |
+| main.go:29:6:29:6 | (def@29:6) | b |
+| main.go:31:9:31:9 | (phi@31:9) | a |
+| main.go:31:9:31:9 | (phi@31:9) | b |
+| main.go:34:11:34:11 | (def@34:11) | x |
+| main.go:39:2:39:2 | (def@39:2) | x |
+| main.go:40:2:40:4 | (def@40:2) | ptr |
+| main.go:48:2:48:7 | (def@48:2) | result |
+| main.go:52:14:52:19 | (def@52:14) | result |
+| main.go:57:6:57:6 | (def@57:6) | x |
+| main.go:58:6:58:6 | (phi@58:6) | x |
+| main.go:59:3:59:3 | (def@59:3) | x |
+| main.go:63:2:63:2 | (def@63:2) | y |
+| main.go:64:6:64:6 | (def@64:6) | i |
+| main.go:64:16:64:18 | (def@64:16) | i |
+| main.go:65:6:65:6 | (phi@65:6) | i |
+| main.go:65:6:65:6 | (phi@65:6) | y |
+| main.go:68:3:68:3 | (def@68:3) | y |
+| main.go:73:6:73:6 | (def@73:6) | i |
+| main.go:73:16:73:18 | (def@73:16) | i |
+| main.go:74:3:74:3 | (def@74:3) | z |
+| main.go:74:3:74:3 | (phi@74:3) | i |
+| main.go:82:25:82:25 | (def@82:25) | b |
+| main.go:83:2:83:2 | (def@83:2) | x |
+| main.go:84:5:84:5 | (def@84:5) | a |
+| main.go:95:22:95:28 | (def@95:22) | wrapper |
+| main.go:95:22:95:28 | (def@95:22).s | wrapper.s |
+| main.go:96:2:96:2 | (def@96:2) | x |
+| main.go:97:2:99:3 | (capture@97:2) | x |
+| main.go:98:3:98:3 | (def@98:3) | x |
+| main.go:112:3:112:3 | (def@112:3) | p |
+| main.go:114:3:114:3 | (def@114:3) | p |
+| main.go:117:2:117:2 | (phi@117:2) | p |
+| main.go:117:2:117:2 | (phi@117:2).a | p.a |
+| main.go:117:2:117:2 | (phi@117:2).b | p.b |
+| main.go:117:2:117:2 | (phi@117:2).b.a | p.b.a |
+| main.go:117:2:117:2 | (phi@117:2).c | p.c |

ql/test/library-tests/semmle/go/dataflow/SSA/SsaWithFields.ql

@@ -0,0 +1,4 @@
+import go
+
+from SsaWithFields v
+select v, v.getQualifiedName()

ql/test/library-tests/semmle/go/dataflow/SSA/VarDefs.expected

@@ -28,7 +28,20 @@
 | main.go:83:2:83:2 | assignment to x | main.go:83:2:83:2 | x | main.go:83:7:83:8 | 23 |
 | main.go:84:2:84:2 | assignment to x | main.go:83:2:83:2 | x | main.go:84:9:84:12 | ...+... |
 | main.go:84:5:84:5 | assignment to a | main.go:82:18:82:18 | a | main.go:84:15:84:15 | x |
-| main.go:90:15:90:16 | initialization of cb | main.go:90:15:90:16 | cb | main.go:90:15:90:16 | argument corresponding to cb |
-| main.go:92:22:92:28 | initialization of wrapper | main.go:92:22:92:28 | wrapper | main.go:92:22:92:28 | argument corresponding to wrapper |
-| main.go:93:2:93:2 | assignment to x | main.go:93:2:93:2 | x | main.go:93:7:93:7 | 0 |
-| main.go:95:3:95:3 | assignment to x | main.go:93:2:93:2 | x | main.go:95:7:95:7 | 1 |
+| main.go:93:15:93:16 | initialization of cb | main.go:93:15:93:16 | cb | main.go:93:15:93:16 | argument corresponding to cb |
+| main.go:95:22:95:28 | initialization of wrapper | main.go:95:22:95:28 | wrapper | main.go:95:22:95:28 | argument corresponding to wrapper |
+| main.go:96:2:96:2 | assignment to x | main.go:96:2:96:2 | x | main.go:96:7:96:7 | 0 |
+| main.go:98:3:98:3 | assignment to x | main.go:96:2:96:2 | x | main.go:98:7:98:7 | 1 |
+| main.go:110:6:110:6 | assignment to p | main.go:110:6:110:6 | p | main.go:110:6:110:6 | zero value for p |
+| main.go:112:3:112:3 | assignment to p | main.go:110:6:110:6 | p | main.go:112:7:112:24 | composite literal |
+| main.go:112:9:112:9 | init of 2 | main.go:104:2:104:2 | a | main.go:112:9:112:9 | 2 |
+| main.go:112:12:112:18 | init of composite literal | main.go:105:2:105:2 | b | main.go:112:12:112:18 | composite literal |
+| main.go:112:14:112:14 | init of 1 | main.go:89:2:89:2 | a | main.go:112:14:112:14 | 1 |
+| main.go:112:17:112:17 | init of 5 | main.go:90:2:90:2 | b | main.go:112:17:112:17 | 5 |
+| main.go:112:21:112:23 | init of 'n' | main.go:106:2:106:2 | c | main.go:112:21:112:23 | 'n' |
+| main.go:114:3:114:3 | assignment to p | main.go:110:6:110:6 | p | main.go:114:7:114:24 | composite literal |
+| main.go:114:9:114:9 | init of 3 | main.go:104:2:104:2 | a | main.go:114:9:114:9 | 3 |
+| main.go:114:12:114:18 | init of composite literal | main.go:105:2:105:2 | b | main.go:114:12:114:18 | composite literal |
+| main.go:114:14:114:14 | init of 4 | main.go:89:2:89:2 | a | main.go:114:14:114:14 | 4 |
+| main.go:114:17:114:17 | init of 5 | main.go:90:2:90:2 | b | main.go:114:17:114:17 | 5 |
+| main.go:114:21:114:23 | init of '2' | main.go:106:2:106:2 | c | main.go:114:21:114:23 | '2' |

ql/test/library-tests/semmle/go/dataflow/SSA/VarUses.expected

@@ -26,6 +26,15 @@
 | main.go:82:25:82:25 | implicit read of b | main.go:82:25:82:25 | b |
 | main.go:84:9:84:9 | x | main.go:83:2:83:2 | x |
 | main.go:84:15:84:15 | x | main.go:83:2:83:2 | x |
-| main.go:94:2:94:8 | wrapper | main.go:92:22:92:28 | wrapper |
-| main.go:94:2:94:10 | selection of s | main.go:92:38:92:38 | s |
-| main.go:97:9:97:9 | x | main.go:93:2:93:2 | x |
+| main.go:97:2:97:8 | wrapper | main.go:95:22:95:28 | wrapper |
+| main.go:97:2:97:10 | selection of s | main.go:95:38:95:38 | s |
+| main.go:100:9:100:9 | x | main.go:96:2:96:2 | x |
+| main.go:117:2:117:2 | p | main.go:110:6:110:6 | p |
+| main.go:117:2:117:4 | selection of b | main.go:105:2:105:2 | b |
+| main.go:119:12:119:12 | p | main.go:110:6:110:6 | p |
+| main.go:119:12:119:14 | selection of a | main.go:104:2:104:2 | a |
+| main.go:119:17:119:17 | p | main.go:110:6:110:6 | p |
+| main.go:119:17:119:19 | selection of b | main.go:105:2:105:2 | b |
+| main.go:119:17:119:21 | selection of a | main.go:89:2:89:2 | a |
+| main.go:119:24:119:24 | p | main.go:110:6:110:6 | p |
+| main.go:119:24:119:26 | selection of c | main.go:106:2:106:2 | c |

ql/test/library-tests/semmle/go/dataflow/SSA/main.go

@@ -85,7 +85,10 @@ func multiRes() (a int, b float32) {
 	return
 }
 
-type s struct{}
+type s struct {
+	a int
+	b int
+}
 
 func (*s) foo(cb func()) {}
 
@@ -96,3 +99,22 @@ func updateInClosure(wrapper struct{ s }) int {
 	})
 	return x
 }
+
+type t struct {
+	a int
+	b s
+	c rune
+}
+
+func ssaPhi() {
+	var p t
+	if cond() {
+		p = t{2, s{1, 5}, 'n'}
+	} else {
+		p = t{3, s{4, 5}, '2'}
+	}
+
+	p.b.foo(func() {})
+
+	fmt.Print(p.a, p.b.a, p.c)
+}

ql/test/query-tests/Security/CWE-601/BadRedirectCheck/BadRedirectCheck.expected

@@ -0,0 +1,5 @@
+| BadRedirectCheck.go:4:23:4:37 | ...==... | This expression checks '$@' for a leading slash but checks do not exist for both '/' and '\\' in the second position. | BadRedirectCheck.go:3:18:3:22 | (def@3:18) | redir |
+| cves.go:11:26:11:38 | ...==... | This expression checks '$@' for a leading slash but checks do not exist for both '/' and '\\' in the second position. | cves.go:10:22:10:24 | (def@10:22) | url |
+| cves.go:22:6:22:37 | call to HasPrefix | This expression checks '$@' for a leading slash but checks do not exist for both '/' and '\\' in the second position. | cves.go:21:2:21:9 | (def@21:2) | redirect |
+| cves.go:29:6:29:37 | call to HasPrefix | This expression checks '$@' for a leading slash but checks do not exist for both '/' and '\\' in the second position. | cves.go:28:2:28:9 | (def@28:2) | redirect |
+| main.go:8:7:8:38 | call to HasPrefix | This expression checks '$@' for a leading slash but checks do not exist for both '/' and '\\' in the second position. | main.go:5:19:5:26 | (def@5:19) | redirect |

ql/test/query-tests/Security/CWE-601/BadRedirectCheck/BadRedirectCheck.go

@@ -0,0 +1,8 @@
+package main
+
+func sanitizeUrl(redir string) string {
+	if len(redir) > 0 && redir[0] == '/' {
+		return redir
+	}
+	return "/"
+}

ql/test/query-tests/Security/CWE-601/BadRedirectCheck/BadRedirectCheck.qlref

@@ -0,0 +1 @@
+Security/CWE-601/BadRedirectCheck.ql

ql/test/query-tests/Security/CWE-601/BadRedirectCheck/BadRedirectCheckGood.go

@@ -0,0 +1,8 @@
+package main
+
+func sanitizeUrl1(redir string) string {
+	if len(redir) > 1 && redir[0] == '/' && redir[1] != '/' && redir[1] != '\\' {
+		return redir
+	}
+	return "/"
+}

ql/test/query-tests/Security/CWE-601/BadRedirectCheck/cves.go

@@ -0,0 +1,32 @@
+package main
+
+import (
+	"net/http"
+	"strings"
+)
+
+// CVE-2018-15178
+// Code from github.com/gogs/gogs
+func isValidRedirect(url string) bool {
+	return len(url) >= 2 && url[0] == '/' && url[1] != '/' // NOT OK
+}
+
+func isValidRedirect1(url string) bool {
+	return len(url) >= 2 && url[0] == '/' && url[1] != '/' && url[1] != '\\' // OK
+}
+
+// CVE-2017-1000070 (both vulnerable!)
+// Code from github.com/bitly/oauth2_proxy
+func OAuthCallback(rw http.ResponseWriter, req *http.Request) {
+	redirect := req.Form.Get("state")
+	if !strings.HasPrefix(redirect, "/") { // NOT OK
+		redirect = "/"
+	}
+}
+
+func OAuthCallback1(rw http.ResponseWriter, req *http.Request) {
+	redirect := req.Form.Get("state")
+	if !strings.HasPrefix(redirect, "/") || strings.HasPrefix(redirect, "//") { // NOT OK
+		redirect = "/"
+	}
+}

ql/test/query-tests/Security/CWE-601/BadRedirectCheck/main.go

@@ -0,0 +1,23 @@
+package main
+
+import "strings"
+
+func isValidRedir(redirect string) bool {
+	switch {
+	// Not OK: does not check for '/\'
+	case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//"):
+		return true
+	default:
+		return false
+	}
+}
+
+func isValidRedir1(redirect string) bool {
+	switch {
+	// OK
+	case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") && !strings.HasPrefix(redirect, "/\\"):
+		return true
+	default:
+		return false
+	}
+}