From ee2804dfb1731fdfb770522adabb534832857d46 Mon Sep 17 00:00:00 2001
From: Slavomir <gagliardetto@users.noreply.github.com>
Date: Fri, 17 Jul 2020 11:01:25 +0300
Subject: [PATCH] Improve comments

---
 .../CWE-352/ConstantOauth2State.ql             | 18 +++++++++++++-----
 ql/src/semmle/go/frameworks/Stdlib.qll         |  4 ++++
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/ql/src/experimental/CWE-352/ConstantOauth2State.ql b/ql/src/experimental/CWE-352/ConstantOauth2State.ql
index 46eed76acaf..96c095f637e 100644
--- a/ql/src/experimental/CWE-352/ConstantOauth2State.ql
+++ b/ql/src/experimental/CWE-352/ConstantOauth2State.ql
@@ -58,7 +58,7 @@ class FlowToPrint extends DataFlow::Configuration {
 }
 
 /** Holds if the provided CallNode's result flows to a Printer call as argument. */
-predicate flowsToPrinter(DataFlow::CallNode authCodeURLCall) {
+predicate resultFlowsToPrinter(DataFlow::CallNode authCodeURLCall) {
   exists(FlowToPrint cfg, DataFlow::PathNode source, DataFlow::PathNode sink |
     cfg.hasFlowPath(source, sink) and
     cfg.isSource(source.getNode(), authCodeURLCall)
@@ -78,6 +78,17 @@ predicate rootContainsCallToStdinScanner(DataFlow::CallNode authCodeURLCall) {
   )
 }
 
+/**
+ * Holds if the authCodeURLCall seems to be done within a terminal
+ * because there are calls to a Printer (fmt.Println and similar),
+ * and a call to a Scanner (fmt.Scan and similar),
+ * all of which are typically done within a terminal session.
+ */
+predicate seemsLikeDoneWithinATerminal(DataFlow::CallNode authCodeURLCall) {
+  resultFlowsToPrinter(authCodeURLCall) and
+  rootContainsCallToStdinScanner(authCodeURLCall)
+}
+
 from
   ConstantStateFlowConf cfg, DataFlow::PathNode source, DataFlow::PathNode sink,
   DataFlow::CallNode sinkCall
@@ -85,9 +96,6 @@ where
   cfg.hasFlowPath(source, sink) and
   cfg.isSink(sink.getNode(), sinkCall) and
   // Exclude cases that seem to be oauth flows done from within a terminal:
-  not (
-    flowsToPrinter(sinkCall) and
-    rootContainsCallToStdinScanner(sinkCall)
-  )
+  not seemsLikeDoneWithinATerminal(sinkCall)
 select sink.getNode(), source, sink, "Using a constant $@ to create oauth2 URLs.", source.getNode(),
   "state string"
diff --git a/ql/src/semmle/go/frameworks/Stdlib.qll b/ql/src/semmle/go/frameworks/Stdlib.qll
index 868318db7af..29d58b89664 100644
--- a/ql/src/semmle/go/frameworks/Stdlib.qll
+++ b/ql/src/semmle/go/frameworks/Stdlib.qll
@@ -154,6 +154,10 @@ module Fmt {
   class FScannerCall extends DataFlow::CallNode {
     FScannerCall() { this.getTarget() instanceof FScanner }
 
+    /**
+     * Returns the node corresponding to the io.Reader
+     * argument provided in the call.
+     */
     DataFlow::Node getReader() { result = this.getArgument(0) }
   }
 }