Skip to content

Commit 16bacc7

Browse files
committed
QL: ql/unqueryable-code query
1 parent c76b6d1 commit 16bacc7

File tree

2 files changed

+62
-8
lines changed

2 files changed

+62
-8
lines changed

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

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ private AstNode publicApi() {
3030
*/
3131
private AstNode queryPredicate() {
3232
// result = query relation that is "transitively" imported by a .ql file.
33-
PathProblemQuery::importsQueryRelation(result).asFile().getExtension() = "ql"
33+
// PathProblemQuery::importsQueryRelation(result).asFile().getExtension() = "ql"
34+
// any query predicate. Query predicates are usually meant to be used.
35+
result.(Predicate).hasAnnotation("query")
3436
or
3537
// the from-where-select
3638
result instanceof Select
@@ -208,8 +210,9 @@ private AstNode benign() {
208210
result instanceof Comment or
209211
not exists(result.toString()) or // <- invalid code
210212
// cached-stages pattern
211-
result.(Module).getAMember().(ClasslessPredicate).getName() = "forceStage" or
212-
result.(ClasslessPredicate).getName() = "forceStage" or
213+
result.(Module).getAMember().(ClasslessPredicate).getName() =
214+
["forceStage", "forceCachingInSameStageforceCachingInSameStage"] or
215+
result.(ClasslessPredicate).getName() = ["forceStage", "forceCachingInSameStage"] or
213216
result.getLocation().getFile().getBaseName() = "Caching.qll" or
214217
// sometimes contains dead code - ignore
215218
result.getLocation().getFile().getRelativePath().matches("%/tutorials/%") or
@@ -243,20 +246,56 @@ private AstNode queryable() {
243246
or
244247
result instanceof TopLevel // toplevel is always alive.
245248
or
249+
result = hackyShouldBeTreatedAsAlive()
250+
or
251+
// The below prevents the query from being too loud. The files below contain a lot of unqueryable code.
252+
// 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).
253+
result
254+
.getLocation()
255+
.getFile()
256+
.getBaseName()
257+
.matches(["DataFlowImpl", "SsaImplCommon", "FlowSummary"] + "%")
258+
or
246259
// recurisve cases
247260
result = aliveStep(queryable())
248261
}
249262

263+
// The benign cases are mostly
264+
private AstNode benignUnqueryable() {
265+
result = benign() or
266+
// cached-stages pattern
267+
// sometimes contains dead code - ignore
268+
result.(Module).getName() = "Debugging" or
269+
result.getLocation().getFile() = benignUnqueryableFile()
270+
}
271+
272+
pragma[noinline]
273+
private File benignUnqueryableFile() {
274+
result.getAbsolutePath().matches("%/explore/%") or
275+
result.getRelativePath().matches("%/tutorials/%") or
276+
result.getRelativePath().matches("%/experimental/%") or
277+
result.getBaseName() =
278+
[
279+
"Expr.qll", "TypeScript.qll", "YAML.qll", "Tokens.qll", "Instruction.qll", "Persistence.qll",
280+
"ES2015Modules.qll"
281+
] or // lots of classes that exist for completeness
282+
result.getBaseName() = ["CachedStages.qll", "Caching.qll", "tutorial.qll"] or
283+
result.getBaseName() = "PrettyPrintAst.qll" or // it's dead code, but seems intentional
284+
result.getBaseName() = ["CryptoAlgorithmNames.qll", "SensitiveDataHeuristics.qll"] or // not all langs use all the things
285+
// some more identical files
286+
result.getBaseName() = "ReachableBlock.qll" or
287+
// QL-for-QL tests contain plenty of unqueryable code on purpose
288+
result.getAbsolutePath().matches("%/ql/ql/test%")
289+
}
290+
250291
/**
251292
* Gets an AstNode that does not affect any query result.
252293
* Is interresting as an quick-eval target to investigate dead code.
253294
* (It is intentional that this predicate is a result of this predicate).
254295
*/
255-
AstNode unQueryable(string msg) {
296+
AstNode unQueryable() {
256297
not result = queryable() and
257298
not result = deprecated() and
258-
not result = benign() and
259-
not result.getParent() = any(AstNode node | not node = queryable()) and
260-
msg = result.getLocation().getFile().getBaseName() and
261-
result.getLocation().getFile().getAbsolutePath().matches("%/javascript/%")
299+
not result = benignUnqueryable() and
300+
not result.getParent() = any(AstNode node | not node = queryable())
262301
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/**
2+
* @name Unqueryable code
3+
* @description Code that cannot affect the outcome of any query is suspicous.
4+
* @kind problem
5+
* @problem.severity recommendation
6+
* @id ql/unqueryable-code
7+
* @precision high
8+
*/
9+
10+
import ql
11+
import codeql_ql.style.DeadCodeQuery
12+
13+
from AstNode node
14+
where node = unQueryable()
15+
select node, "Code cannot affect the outcome of any query."

0 commit comments

Comments
 (0)