Fine tune the query and update qldoc

Author: luchua-bc

java/ql/src/experimental/Security/CWE/CWE-200/AndroidWebResourceResponse.qll

@@ -21,21 +21,19 @@ class WebResourceResponse extends RefType {
 class ShouldInterceptRequestMethod extends Method {
   ShouldInterceptRequestMethod() {
     this.hasName("shouldInterceptRequest") and
-    this.getDeclaringType().getASupertype*() instanceof TypeWebViewClient and
-    this.getReturnType() instanceof WebResourceResponse
+    this.getDeclaringType().getASupertype*() instanceof TypeWebViewClient
   }
 }
 
 /** A method call to `setWebViewClient` of `WebView`. */
 class SetWebViewClientMethodAccess extends MethodAccess {
   SetWebViewClientMethodAccess() {
     this.getMethod().hasName("setWebViewClient") and
-    this.getMethod().getDeclaringType().getASupertype*() instanceof TypeWebView and
-    this.getMethod().getParameterType(0) instanceof TypeWebViewClient
+    this.getMethod().getDeclaringType().getASupertype*() instanceof TypeWebView
   }
 }
 
-/** A sink representing a constructor call of `WebResourceResponse` in Android `WebViewClient`. */
+/** A sink representing the data argument of a call to the constructor of `WebResourceResponse`. */
 class WebResourceResponseSink extends DataFlow::Node {
   WebResourceResponseSink() {
     exists(ConstructorCall cc |
@@ -50,7 +48,8 @@ class WebResourceResponseSink extends DataFlow::Node {
 }
 
 /**
- * Value step from a fetching url call of `WebView` to `WebViewClient`.
+ * A value step from the URL argument of `WebView::loadUrl` to the URL parameter of
+ * `WebViewClient::shouldInterceptRequest`.
  */
 private class FetchUrlStep extends AdditionalValueStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
@@ -63,20 +62,20 @@ private class FetchUrlStep extends AdditionalValueStep {
         lma.getMethod() = lm and
         lma.getQualifier().getType() = sma.getQualifier().getType() and
         pred.asExpr() = lma.getArgument(0) and
-        succ.asExpr() = im.getParameter(1).getAnAccess()
+        succ.asParameter() = im.getParameter(1)
       )
     )
   }
 }
 
 /** Value/taint steps relating to url loading and file reading in an Android application. */
-private class LoadUrlSource extends SummaryModelCsv {
+private class LoadUrlSummaries extends SummaryModelCsv {
   override predicate row(string row) {
     row =
       [
         "java.io;FileInputStream;true;FileInputStream;;;Argument[0];Argument[-1];taint",
-        "android.net;Uri;false;getPath;;;Argument[0];ReturnValue;value",
-        "android.webkit;WebResourceRequest;false;getUrl;;;Argument[-1];ReturnValue;value"
+        "android.net;Uri;false;getPath;;;Argument[0];ReturnValue;taint",
+        "android.webkit;WebResourceRequest;false;getUrl;;;Argument[-1];ReturnValue;taint"
       ]
   }
 }

java/ql/src/experimental/Security/CWE/CWE-200/InsecureWebResourceResponse.qhelp

@@ -3,36 +3,36 @@
   "qhelp.dtd">
 <qhelp>
 <overview>
-<p>Android provides a <code>WebResourceResponse</code> API, which is a <code>WebView</code> class that
-allows an Android application to behave as a web server by handling requests of popular protocols such
-as <code>http(s)</code>, <code>file</code>, as well as <code>javascript</code>; and returning a response
-(including status code, content type, content encoding, headers and the response body). Improper
-implementation with insufficient input validation can lead to leaking of sensitive configuration file
-or user data because requests could refer to paths intended to be application-private.
+<p>Android provides a <code>WebResourceResponse</code> class, which allows an Android application to behave
+as a web server by handling requests of popular protocols such as <code>http(s)</code>, <code>file</code>,
+as well as <code>javascript</code>; and returning a response (including status code, content type, content
+encoding, headers and the response body). Improper implementation with insufficient input validation can lead
+to leakage of sensitive configuration files or user data because requests could refer to paths intended to be
+application-private.
 </p>
 </overview>
 
 <recommendation>
 <p>
-Unsanitized user provided url must not be used to serve a response directly. When handling a request,
-always validate that the file path is not the receiver's protected directory. Alternatively the Android
-API <code>WebViewAssetLoader</code> can be used, which safely processes data from resources, assets or
-a predefined directory.
+Unsanitized user-provided URLs must not be used to serve a response directly. When handling a request,
+always validate that the requested file path is not in the receiver's protected directory. Alternatively
+the Android class <code>WebViewAssetLoader</code> can be used, which safely processes data from resources,
+assets or a predefined directory.
 </p>
 </recommendation>
 
 <example>
 <p>
-The following examples show a bad situation and two good situations respectively. In the bad situation, a
-response is served without path validation. In the good situation, a response is either served with path
+The following examples show a bad scenario and two good scenarios respectively. In the bad scenario, a
+response is served without path validation. In the good scenario, a response is either served with path
 validation or through the safe <code>WebViewAssetLoader</code> implementation.
 </p>
 <sample src="InsecureWebResourceResponse.java" />
 </example>
 
 <references>
 <li>
-Google:
+Oversecured:
 <a href="https://blog.oversecured.com/Android-Exploring-vulnerabilities-in-WebResourceResponse/">Android: Exploring vulnerabilities in WebResourceResponse</a>.
 </li>
 <li>

java/ql/src/experimental/Security/CWE/CWE-200/InsecureWebResourceResponse.ql

@@ -1,7 +1,7 @@
 /**
  * @name Insecure Android WebView Resource Response
- * @description Insecure implementation of Android WebResourceResponse intercepts malicious app requests
- *              and return arbitrary sensitive content.
+ * @description An insecure implementation of Android `WebResourceResponse` may lead to leakage of arbitrary
+ *               sensitive content.
  * @kind path-problem
  * @id java/insecure-webview-resource-response
  * @problem.severity error
@@ -29,5 +29,5 @@ class InsecureWebResourceResponseConfig extends TaintTracking::Configuration {
 
 from DataFlow::PathNode source, DataFlow::PathNode sink, InsecureWebResourceResponseConfig conf
 where conf.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "Leaking arbitrary content in Android from $@.", source.getNode(),
-  "this user input"
+select sink.getNode(), source, sink, "Leaking arbitrary content in Android from $@.",
+  source.getNode(), "this user input"