From f5e969059466edcd5c1bf44b86344cd03ad84db2 Mon Sep 17 00:00:00 2001
From: luchua-bc <shengxin.canada@gmail.com>
Date: Thu, 15 Oct 2020 10:52:30 +0000
Subject: [PATCH] Update the doc comments

---
 .../CWE/CWE-749/UnsafeAndroidAccess.qhelp     |  2 +-
 .../CWE/CWE-749/UnsafeAndroidAccess.ql        | 22 +++++++++++++++----
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/java/ql/src/experimental/Security/CWE/CWE-749/UnsafeAndroidAccess.qhelp b/java/ql/src/experimental/Security/CWE/CWE-749/UnsafeAndroidAccess.qhelp
index 1ef21e0c3e1..3ac8ec67e59 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-749/UnsafeAndroidAccess.qhelp
+++ b/java/ql/src/experimental/Security/CWE/CWE-749/UnsafeAndroidAccess.qhelp
@@ -13,7 +13,7 @@
   </overview>
 
   <recommendation>
-    <p>Only allow trusted web content to be displayed in WebViews when JavaScript is enabled. Disallow cross-origin resource access in WebSetting to reduce the attack surface .</p>
+    <p>Only allow trusted web content to be displayed in WebViews when JavaScript is enabled. Disallow cross-origin resource access in WebSetting to reduce the attack surface.</p>
   </recommendation>
 
   <example>
diff --git a/java/ql/src/experimental/Security/CWE/CWE-749/UnsafeAndroidAccess.ql b/java/ql/src/experimental/Security/CWE/CWE-749/UnsafeAndroidAccess.ql
index bbf788bc10d..204b030a17c 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-749/UnsafeAndroidAccess.ql
+++ b/java/ql/src/experimental/Security/CWE/CWE-749/UnsafeAndroidAccess.ql
@@ -71,7 +71,7 @@ class IntentGetExtraMethodAccess extends MethodAccess {
 }
 
 /**
- * Source of fetching urls
+ * Source of fetching URLs
  */
 class UntrustedResourceSource extends RemoteFlowSource {
   UntrustedResourceSource() {
@@ -84,21 +84,28 @@ class UntrustedResourceSource extends RemoteFlowSource {
 }
 
 /**
- * Holds if `ma` loads url `sink`
+ * Holds if `ma` loads URL `sink`
  */
 predicate fetchResource(FetchResourceMethodAccess ma, Expr sink) { sink = ma.getArgument(0) }
 
 /**
- * Sink of fetching urls
+ * A URL argument to a `loadUrl` or `postUrl` call, considered as a sink.
  */
 class UrlResourceSink extends DataFlow::ExprNode {
   UrlResourceSink() { fetchResource(_, this.getExpr()) }
 
+  /** Gets the fetch method that fetches this sink URL. */
   FetchResourceMethodAccess getMethodAccess() { fetchResource(result, this.getExpr()) }
 
+  /**
+   * Holds if cross-origin access is enabled for this resource fetch.
+   *
+   * Specifically this looks for code like
+   * `webView.getSettings().setAllow[File|Universal]AccessFromFileURLs(true);`
+   */
   predicate crossOriginAccessEnabled() {
     exists(MethodAccess ma, MethodAccess getSettingsMa |
-      ma.getMethod() instanceof CrossOriginAccessMethod and // Unsafe resource fetching of more severe vulnerabilities
+      ma.getMethod() instanceof CrossOriginAccessMethod and
       ma.getArgument(0).(BooleanLiteral).getBooleanValue() = true and
       ma.getQualifier().(VarAccess).getVariable().getAnAssignedValue() = getSettingsMa and
       getSettingsMa.getMethod() instanceof WebViewGetSettingsMethod and
@@ -107,6 +114,10 @@ class UrlResourceSink extends DataFlow::ExprNode {
     )
   }
 
+  /**
+   * Returns a description of this vulnerability, assuming Javascript is enabled and
+   * the fetched URL is attacker-controlled.
+   */
   string getSinkType() {
     if crossOriginAccessEnabled()
     then result = "user input vulnerable to cross-origin and sensitive resource disclosure attacks"
@@ -114,6 +125,9 @@ class UrlResourceSink extends DataFlow::ExprNode {
   }
 }
 
+/**
+ * Taint configuration tracking flow from untrusted inputs to `loadUrl` or `postUrl` calls.
+ */
 class FetchUntrustedResourceConfiguration extends TaintTracking::Configuration {
   FetchUntrustedResourceConfiguration() { this = "FetchUntrustedResourceConfiguration" }