Skip to content

Commit 93a6710

Browse files
committed
add a QL-for-QL query highlighting some issues with alert-texts
1 parent 338aead commit 93a6710

File tree

1 file changed

+140
-0
lines changed

1 file changed

+140
-0
lines changed
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
/**
2+
* @name Alert messages style violation
3+
* @description Alert message that doesn't follow some aspect of the style guide.
4+
* See the style guide here: https://github.com/github/codeql/blob/main/docs/query-metadata-style-guide.md#alert-messages
5+
* @kind problem
6+
* @problem.severity warning
7+
* @id ql/alert-messages-style-violation
8+
* @precision high
9+
*/
10+
11+
import ql
12+
13+
/** Gets the `index`th part of the select statement. */
14+
private AstNode getSelectPart(Select sel, int index) {
15+
result =
16+
rank[index](AstNode n, Location loc |
17+
(
18+
n.getParent*() = sel.getExpr(_) and loc = n.getLocation()
19+
or
20+
// the strings are behind a predicate call.
21+
exists(Call c, Predicate target |
22+
c.getParent*() = sel.getExpr(_) and loc = c.getLocation()
23+
|
24+
c.getTarget() = target and
25+
(
26+
target.getBody().(ComparisonFormula).getAnOperand() = n
27+
or
28+
exists(ClassPredicate sub | sub.overrides(target) |
29+
sub.getBody().(ComparisonFormula).getAnOperand() = n
30+
)
31+
)
32+
)
33+
)
34+
|
35+
n
36+
order by
37+
loc.getStartLine(), loc.getStartColumn(), loc.getEndLine(), loc.getEndColumn(),
38+
loc.getFile().getRelativePath()
39+
)
40+
}
41+
42+
String shouldHaveFullStop(Select sel) {
43+
result =
44+
max(AstNode str, int i |
45+
str.getParent+() = sel.getExpr(1) and str = getSelectPart(sel, i)
46+
|
47+
str order by i
48+
) and
49+
not result.getValue().matches("%.") and
50+
not result.getValue().matches("%?")
51+
}
52+
53+
String shouldStartCapital(Select sel) {
54+
result =
55+
min(AstNode str, int i |
56+
str.getParent+() = sel.getExpr(1) and str = getSelectPart(sel, i)
57+
|
58+
str order by i
59+
) and
60+
result.getValue().regexpMatch("^[a-z].*")
61+
}
62+
63+
// see https://www.w3.org/WAI/WCAG22/Understanding/link-purpose-in-context.html
64+
String avoidHere(string part) {
65+
part = ["here", "this location"] and // TODO: prefer "this location" of the two.
66+
(
67+
result.getValue().regexpMatch(".*\\b" + part + "\\b.*") and
68+
result = getSelectPart(_, _)
69+
)
70+
}
71+
72+
// see https://www.w3.org/WAI/WCAG22/Understanding/link-purpose-in-context.html
73+
String avoidArticleInLinkText(Select sel) {
74+
result = sel.getExpr((any(int i | i > 1))) and
75+
result = getSelectPart(sel, _) and
76+
result.getValue().regexpMatch("a|an .*")
77+
}
78+
79+
String dontQuoteSubstitutions(Select sel) {
80+
result = getSelectPart(sel, _) and
81+
result.getValue().matches(["%'$@'%", "%\"$@\"%"])
82+
}
83+
84+
// "data" or "taint"
85+
string getQueryKind(Select s) {
86+
exists(TypeExpr sup |
87+
sup = s.getVarDecl(_).getType().(ClassType).getDeclaration().getASuperType() and
88+
sup.getResolvedType().(ClassType).getName() = "Configuration"
89+
|
90+
result = "data" and
91+
sup.getModule().getName() = "DataFlow"
92+
or
93+
result = "taint" and
94+
sup.getModule().getName() = "TaintTracking"
95+
)
96+
}
97+
98+
String wrongFlowsPhrase(Select sel, string kind) {
99+
result = getSelectPart(sel, _) and
100+
kind = getQueryKind(sel) and
101+
(
102+
kind = "data" and
103+
result.getValue().matches(["% depends %", "% depend %"])
104+
or
105+
kind = "taint" and
106+
result.getValue().matches(["% flows to %", "% flow to %"])
107+
)
108+
}
109+
110+
from AstNode node, string msg
111+
where
112+
not node.getLocation().getFile().getAbsolutePath().matches("%/test/%") and
113+
(
114+
node = shouldHaveFullStop(_) and
115+
msg = "Alert message should end with a full stop."
116+
or
117+
node = shouldStartCapital(_) and
118+
msg = "Alert message should start with a capital letter."
119+
or
120+
exists(string part | node = avoidHere(part) |
121+
part = "here" and
122+
msg = "Avoid using the phrase \"" + part + "\" in alert messages."
123+
or
124+
part != "here" and
125+
msg = "Try to avoid using the phrase \"" + part + "\" in alert messages if possible."
126+
)
127+
or
128+
node = avoidArticleInLinkText(_) and
129+
msg = "Avoid starting a link text with an indefinite article."
130+
or
131+
node = dontQuoteSubstitutions(_) and
132+
msg = "Don't quote substitutions in alert messages."
133+
or
134+
node = wrongFlowsPhrase(_, "data") and
135+
msg = "Use \"flows to\" instead of \"depends on\" in taint tracking queries."
136+
or
137+
node = wrongFlowsPhrase(_, "taint") and
138+
msg = "Use \"depends on\" instead of \"flows to\" in data flow queries."
139+
)
140+
select node, msg

0 commit comments

Comments
 (0)