QL: ql/unqueryable-code query

Author: erik-krogh

ql/ql/src/codeql_ql/style/DeadCodeQuery.qll

@@ -30,7 +30,9 @@ private AstNode publicApi() {
  */
 private AstNode queryPredicate() {
   // result = query relation that is "transitively" imported by a .ql file.
-  PathProblemQuery::importsQueryRelation(result).asFile().getExtension() = "ql"
+  // PathProblemQuery::importsQueryRelation(result).asFile().getExtension() = "ql"
+  // any query predicate. Query predicates are usually meant to be used.
+  result.(Predicate).hasAnnotation("query")
   or
   // the from-where-select
   result instanceof Select
@@ -200,8 +202,9 @@ private AstNode benign() {
   result instanceof BlockComment or
   not exists(result.toString()) or // <- invalid code
   // cached-stages pattern
-  result.(Module).getAMember().(ClasslessPredicate).getName() = "forceStage" or
-  result.(ClasslessPredicate).getName() = "forceStage" or
+  result.(Module).getAMember().(ClasslessPredicate).getName() =
+    ["forceStage", "forceCachingInSameStageforceCachingInSameStage"] or
+  result.(ClasslessPredicate).getName() = ["forceStage", "forceCachingInSameStage"] or
   result.getLocation().getFile().getBaseName() = "Caching.qll" or
   // sometimes contains dead code - ignore
   result.getLocation().getFile().getRelativePath().matches("%/tutorials/%") or
@@ -239,16 +242,46 @@ private AstNode queryable() {
   result = aliveStep(queryable())
 }
 
+// The benign cases are mostly
+private AstNode benignUnqueryable() {
+  result = benign() or
+  // cached-stages pattern
+  // sometimes contains dead code - ignore
+  result.(Module).getName() = "Debugging" or
+  result.getLocation().getFile() = benignUnqueryableFile()
+}
+
+pragma[noinline]
+private File benignUnqueryableFile() {
+  result.getAbsolutePath().matches("%/explore/%") or
+  result.getRelativePath().matches("%/tutorials/%") or
+  result.getBaseName() =
+    [
+      "Expr.qll", "TypeScript.qll", "YAML.qll", "Tokens.qll", "Instruction.qll", "Persistence.qll",
+      "ES2015Modules.qll"
+    ] or // lots of classes that exist for completeness
+  result.getBaseName() = ["CachedStages.qll", "Caching.qll", "tutorial.qll"] or
+  result.getBaseName() = "PrettyPrintAst.qll" or // it's dead code, but seems intentional
+  result.getBaseName() = "SensitiveDataHeuristics.qll" or // not all langs use all the things
+  result.getAbsolutePath().matches("%/ql/ql/test%") // QL-for-QL tests contain plenty of unqueryable code on purpose
+}
+
 /**
  * Gets an AstNode that does not affect any query result.
  * Is interresting as an quick-eval target to investigate dead code.
  * (It is intentional that this predicate is a result of this predicate).
  */
-AstNode unQueryable(string msg) {
+AstNode unQueryable() {
   not result = queryable() and
   not result = deprecated() and
-  not result = benign() and
-  not result.getParent() = any(AstNode node | not node = queryable()) and
-  msg = result.getLocation().getFile().getBaseName() and
-  result.getLocation().getFile().getAbsolutePath().matches("%/javascript/%")
+  not result = benignUnqueryable() and
+  not result.getParent() = any(AstNode node | not node = queryable())
+}
+
+AstNode unqueryableLang(string lang) {
+  lang = "/" + ["cpp", "csharp", "java", "javascript", "python", "ruby"] + "/" and
+  result = unQueryable() and
+  result.getLocation().getFile().getAbsolutePath().matches("%" + lang + "%")
 }
+
+int countLang(string lang) { result = strictcount(unqueryableLang(lang)) }

ql/ql/src/queries/style/UnqueryableCode.ql

@@ -0,0 +1,22 @@
+/**
+ * @name Unqueryable code
+ * @description Code that cannot affect the outcome of any query is suspicous.
+ * @kind problem
+ * @problem.severity recommendation
+ * @id ql/unqueryable-code
+ * @precision high
+ */
+
+import ql
+import codeql_ql.style.DeadCodeQuery
+
+from AstNode node
+where
+  node = unQueryable() and
+  // The below prevents the query from being too loud. The files below contain a lot of unqueryable code.
+  // I think some of it is from some languages not using all features of a shared library, but I'm not sure (haven't look much into it).
+  not node.getLocation()
+      .getFile()
+      .getBaseName()
+      .matches(["DataFlowImpl", "SsaImplCommon", "FlowSummary"] + "%")
+select node, "Code cannot affect the outcome of any query."