BadRedirectSanitizer: Bind e to hp

Address doc review comments

change-notes/1.24/analysis-go.md

@@ -10,10 +10,10 @@
 
 | **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. |
-| Bad check of redirect URL (`go/bad-redirect-check`) | correctness, security, external/cwe/cwe-601 | Highlights checks for redirect URLs ensuring they start with `/` but don't check for `//` or `/\`. Results are shown on LGTM by default. |
 
 ## Changes to existing queries
 

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

@@ -3,7 +3,7 @@
 
 <overview>
 <p>
-Redirect URLs should be checked in order to ensure that user input cannot cause a site to redirect
+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

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

@@ -16,18 +16,19 @@ import go
 predicate checksForLeadingSlash(Expr e, ValueEntity v) {
   exists(LogicalExpr le | le = e | checksForLeadingSlash(le.getAnOperand(), v))
   or
-  exists(StringOps::HasPrefix hp |
-    hp.getBaseString().(Read).reads(v) and
-    // ASCII value for '/'
-    (hp.getSubstring().getStringValue() = "/" or hp.getSubstring().getIntValue() = 47)
+  exists(StringOps::HasPrefix hp, DataFlow::Node substr |
+    e = hp.asExpr() and hp.getBaseString() = v.getARead() and hp.getSubstring() = substr
+  |
+    substr.getStringValue() = "/"
+    or
+    substr.getIntValue() = 47 // ASCII value for '/'
   )
 }
 
 predicate checksForSecondSlash(Expr e, ValueEntity v) {
   exists(LogicalExpr le | le = e | checksForSecondSlash(le.getAnOperand(), v))
   or
-  exists(StringOps::HasPrefix hp |
-    hp.getBaseString().(Read).reads(v) and
+  exists(StringOps::HasPrefix hp | e = hp.asExpr() and hp.getBaseString() = v.getARead() |
     hp.getSubstring().getStringValue() = "//"
   )
   or
@@ -42,8 +43,7 @@ predicate checksForSecondSlash(Expr e, ValueEntity v) {
 predicate checksForSecondBackslash(Expr e, ValueEntity v) {
   exists(LogicalExpr le | le = e | checksForSecondBackslash(le.getAnOperand(), v))
   or
-  exists(StringOps::HasPrefix hp |
-    hp.getBaseString().(Read).reads(v) and
+  exists(StringOps::HasPrefix hp | e = hp.asExpr() and hp.getBaseString() = v.getARead() |
     hp.getSubstring().getStringValue() = "/\\"
   )
   or