Skip to content

Commit 5af739d

Browse files
erik-krogherik-krogh
authored
Merge pull request #10413 from erik-krogh/go-followMsg
GO: make the alert messages of taint-tracking queries more consistent
2 parents 9979fa3 + 175d3ac commit 5af739d

File tree

50 files changed

+317
-319
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+317
-319
lines changed

‎go/ql/src/InconsistentCode/ConstantLengthComparison.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,5 +30,5 @@ where
3030
cond.dominates(idx.getBasicBlock()) and
3131
// and that check happens inside the loop body
3232
cond.getCondition().getParent+() = fs
33-
select cond.getCondition(),
34-
"This checks the length against a constant, but it is indexed using a variable $@.", idx, "here"
33+
select cond.getCondition(), "This checks the length against a constant, but it $@.", idx,
34+
"is indexed using a variable"

‎go/ql/src/InconsistentCode/MissingErrorCheck.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,5 +116,5 @@ where
116116
// `deref` dereferences `ptr`
117117
deref.getOperand() = ptr.getAUse()
118118
select deref.getOperand(),
119-
ptr.getSourceVariable() + " may be nil here, because $@ may not have been checked.", err,
120-
err.getSourceVariable().toString()
119+
"$@ may be nil at this dereference because $@ may not have been checked.", ptr,
120+
ptr.getSourceVariable().toString(), err, err.getSourceVariable().toString()

‎go/ql/src/RedundantCode/CompareIdenticalValues.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,4 @@ where
2424
cmp.getAnOperand() = decl.getAReference() and
2525
cmp.getAnOperand() instanceof BasicLit
2626
)
27-
select cmp, "This expression compares $@ to itself.", cmp.getLeftOperand(), "an expression"
27+
select cmp, "This expression compares an $@ to itself.", cmp.getLeftOperand(), "expression"

‎go/ql/src/RedundantCode/DuplicateCondition.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,4 @@ GVN conditionGvn(IfStmt is, int i, Expr e) {
3030

3131
from IfStmt is, Expr e, Expr f, int i, int j
3232
where conditionGvn(is, i, e) = conditionGvn(is, j, f) and i < j
33-
select f, "This condition is a duplicate of $@.", e, "an earlier condition"
33+
select f, "This condition is a duplicate of an $@.", e, "earlier condition"

‎go/ql/src/RedundantCode/DuplicateSwitchCase.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,4 @@ GVN switchCaseGvn(SwitchStmt switch, int i, Expr e) {
2020

2121
from SwitchStmt switch, int i, Expr e, int j, Expr f
2222
where switchCaseGvn(switch, i, e) = switchCaseGvn(switch, j, f) and i < j
23-
select f, "This case is a duplicate of $@.", e, "an earlier case"
23+
select f, "This case is a duplicate of an $@.", e, "earlier case"

‎go/ql/src/RedundantCode/SelfAssignment.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,4 @@ from PotentialSelfAssignment assgn, HashableNode rhs
2323
where
2424
rhs = assgn.getRhs() and
2525
rhs.hash() = assgn.getLhs().(HashableNode).hash()
26-
select assgn, "This statement assigns $@ to itself.", rhs, "an expression"
26+
select assgn, "This statement assigns an $@ to itself.", rhs, "expression"

‎go/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,4 +110,4 @@ from Config c, DataFlow::PathNode source, DataFlow::PathNode sink, string hostPa
110110
where c.hasFlowPath(source, sink) and c.isSource(source.getNode(), hostPart)
111111
select source, source, sink,
112112
"This regular expression has an unescaped dot before '" + hostPart + "', " +
113-
"so it might match more hosts than expected when used $@.", sink, "here"
113+
"so it might match more hosts than expected when $@.", sink, "the regular expression is used"

‎go/ql/src/Security/CWE-020/SuspiciousCharacterInRegexp.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,5 +48,5 @@ class Config extends DataFlow::Configuration {
4848

4949
from Config c, DataFlow::PathNode source, DataFlow::PathNode sink, string report
5050
where c.hasFlowPath(source, sink) and c.isSource(source.getNode(), report)
51-
select source, source, sink, "$@ used $@ contains " + report, source, "A regular expression", sink,
52-
"here"
51+
select source, source, sink, "$@ that is $@ contains " + report, source, "A string literal", sink,
52+
"used as a regular expression"

‎go/ql/src/Security/CWE-022/TaintedPath.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,5 @@ import DataFlow::PathGraph
2121

2222
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
2323
where cfg.hasFlowPath(source, sink)
24-
select sink.getNode(), source, sink, "$@ flows to here and is used in a path.", source.getNode(),
25-
"User-provided value"
24+
select sink.getNode(), source, sink, "This path depends on a $@.", source.getNode(),
25+
"user-provided value"

‎go/ql/src/Security/CWE-078/CommandInjection.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,5 @@ from
1919
CommandInjection::Configuration cfg, CommandInjection::DoubleDashSanitizingConfiguration cfg2,
2020
DataFlow::PathNode source, DataFlow::PathNode sink
2121
where cfg.hasFlowPath(source, sink) or cfg2.hasFlowPath(source, sink)
22-
select sink.getNode(), source, sink, "This command depends on $@.", source.getNode(),
23-
"a user-provided value"
22+
select sink.getNode(), source, sink, "This command depends on a $@.", source.getNode(),
23+
"user-provided value"

‎go/ql/src/Security/CWE-078/StoredCommand.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,5 @@ import DataFlow::PathGraph
1717

1818
from StoredCommand::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
1919
where cfg.hasFlowPath(source, sink)
20-
select sink.getNode(), source, sink, "This command depends on $@.", source.getNode(),
21-
"a stored value"
20+
select sink.getNode(), source, sink, "This command depends on a $@.", source.getNode(),
21+
"stored value"

‎go/ql/src/Security/CWE-079/ReflectedXss.ql

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,8 @@ where
2424
(
2525
exists(string kind | kind = sink.getNode().(SharedXss::Sink).getSinkKind() |
2626
kind = "rawtemplate" and
27-
msg =
28-
"Cross-site scripting vulnerability due to $@. This template argument is instantiated raw $@." and
29-
part = "here"
27+
msg = "Cross-site scripting vulnerability due to $@. The value is $@." and
28+
part = "instantiated as a raw template"
3029
)
3130
or
3231
not exists(sink.getNode().(SharedXss::Sink).getSinkKind()) and

‎go/ql/src/Security/CWE-089/SqlInjection.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,5 @@ import DataFlow::PathGraph
1717

1818
from SqlInjection::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
1919
where cfg.hasFlowPath(source, sink)
20-
select sink.getNode(), source, sink, "This query depends on $@.", source.getNode(),
21-
"a user-provided value"
20+
select sink.getNode(), source, sink, "This query depends on a $@.", source.getNode(),
21+
"user-provided value"

‎go/ql/src/Security/CWE-117/LogInjection.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,5 @@ import DataFlow::PathGraph
1717

1818
from LogInjection::Configuration c, DataFlow::PathNode source, DataFlow::PathNode sink
1919
where c.hasFlowPath(source, sink)
20-
select sink, source, sink, "This log write receives unsanitized user input from $@.",
21-
source.getNode(), "here"
20+
select sink.getNode(), source, sink, "Log entry depends on a $@.", source.getNode(),
21+
"user-provided value"

‎go/ql/src/Security/CWE-190/AllocationSizeOverflow.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,5 @@ where
2222
cfg.hasFlowPath(source, sink) and
2323
cfg.isSink(sink.getNode(), allocsz)
2424
select sink, source, sink,
25-
"This operation, which is used in an $@, involves a potentially large $@ and might overflow.",
26-
allocsz, "allocation", source, "value"
25+
"This operation, which is used in an $@, involves a $@ and might overflow.", allocsz,
26+
"allocation", source, "potentially large value"

‎go/ql/src/Security/CWE-209/StackTraceExposure.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,5 +77,5 @@ class StackTraceExposureConfig extends TaintTracking::Configuration {
7777
from StackTraceExposureConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
7878
where cfg.hasFlowPath(source, sink)
7979
select sink.getNode(), source, sink,
80-
"Stack trace information from $@ may be exposed to an external user here.", source.getNode(),
81-
"here"
80+
"HTTP response depends on $@ and may be exposed to an external user.", source.getNode(),
81+
"stack trace information"

‎go/ql/src/Security/CWE-312/CleartextLogging.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,5 @@ import DataFlow::PathGraph
1919

2020
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
2121
where cfg.hasFlowPath(source, sink)
22-
select sink.getNode(), source, sink, "Sensitive data returned by $@ is logged here.",
23-
source.getNode(), source.getNode().(Source).describe()
22+
select sink.getNode(), source, sink, "$@ flows to a logging call.", source.getNode(),
23+
"Sensitive data returned by " + source.getNode().(Source).describe()

‎go/ql/src/Security/CWE-601/OpenUrlRedirect.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,5 @@ where
2424
// this excludes flow from safe parts of request URLs, for example the full URL when the
2525
// doing a redirect from `http://<path>` to `https://<path>`
2626
not scfg.hasFlow(_, sink.getNode())
27-
select sink.getNode(), source, sink, "Untrusted URL redirection due to $@.", source.getNode(),
27+
select sink.getNode(), source, sink, "Untrusted URL redirection depends on a $@.", source.getNode(),
2828
"user-provided value"

‎go/ql/src/Security/CWE-643/XPathInjection.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,5 @@ predicate isStringOrByte(DataFlow::PathNode node) {
2424

2525
from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
2626
where config.hasFlowPath(source, sink) and isStringOrByte(sink)
27-
select sink.getNode(), source, sink, "$@ flows to here and is used in an XPath expression.",
28-
source.getNode(), "User-provided value"
27+
select sink.getNode(), source, sink, "XPath expression depends on a $@.", source.getNode(),
28+
"user-provided value"

‎go/ql/src/Security/CWE-918/RequestForgery.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,5 @@ where
2323
request = sink.getNode().(Sink).getARequest() and
2424
// this excludes flow from safe parts of request URLs, for example the full URL
2525
not scfg.hasFlow(_, sink.getNode())
26-
select request, source, sink, "The $@ of this request depends on $@.", sink.getNode(),
27-
sink.getNode().(Sink).getKind(), source, "a user-provided value"
26+
select request, source, sink, "The $@ of this request depends on a $@.", sink.getNode(),
27+
sink.getNode().(Sink).getKind(), source, "user-provided value"

‎go/ql/src/experimental/CWE-090/LDAPInjection.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,5 @@ import DataFlow::PathGraph
1515

1616
from LdapInjectionConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
1717
where config.hasFlowPath(source, sink)
18-
select sink.getNode(), source, sink, "LDAP query parameter is derived from $@.", source.getNode(),
19-
"a user-provided value"
18+
select sink.getNode(), source, sink, "LDAP query parameter depends on a $@.", source.getNode(),
19+
"user-provided value"

‎go/ql/src/experimental/CWE-840/ConditionalBypass.ql

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,5 @@ where
3535
rhs.getNode().asExpr() = c.getRightOperand() and
3636
config.hasFlowPath(lhsSource, lhs) and
3737
lhs.getNode().asExpr() = c.getLeftOperand()
38-
select c,
39-
"This comparison compares user-controlled values from $@ and $@, and hence can be bypassed.",
40-
lhsSource, "here", rhsSource, "here"
38+
select c, "This comparison of a $@ with another $@ can be bypassed by a malicious user.",
39+
lhsSource.getNode(), "user-controlled value", rhsSource.getNode(), "user-controlled value"

‎go/ql/test/experimental/CWE-090/LDAPInjection.expected

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,14 @@ nodes
4040
| LDAPInjection.go:81:25:81:33 | untrusted | semmle.label | untrusted |
4141
subpaths
4242
#select
43-
| LDAPInjection.go:59:3:59:11 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:59:3:59:11 | untrusted | LDAP query parameter is derived from $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | a user-provided value |
44-
| LDAPInjection.go:61:3:61:51 | ...+... | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:61:3:61:51 | ...+... | LDAP query parameter is derived from $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | a user-provided value |
45-
| LDAPInjection.go:62:3:62:33 | slice literal | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:62:3:62:33 | slice literal | LDAP query parameter is derived from $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | a user-provided value |
46-
| LDAPInjection.go:66:3:66:11 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:66:3:66:11 | untrusted | LDAP query parameter is derived from $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | a user-provided value |
47-
| LDAPInjection.go:68:3:68:51 | ...+... | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:68:3:68:51 | ...+... | LDAP query parameter is derived from $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | a user-provided value |
48-
| LDAPInjection.go:69:3:69:33 | slice literal | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:69:3:69:33 | slice literal | LDAP query parameter is derived from $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | a user-provided value |
49-
| LDAPInjection.go:73:3:73:11 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:73:3:73:11 | untrusted | LDAP query parameter is derived from $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | a user-provided value |
50-
| LDAPInjection.go:75:3:75:51 | ...+... | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:75:3:75:51 | ...+... | LDAP query parameter is derived from $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | a user-provided value |
51-
| LDAPInjection.go:76:3:76:33 | slice literal | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:76:3:76:33 | slice literal | LDAP query parameter is derived from $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | a user-provided value |
52-
| LDAPInjection.go:80:22:80:30 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:80:22:80:30 | untrusted | LDAP query parameter is derived from $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | a user-provided value |
53-
| LDAPInjection.go:81:25:81:33 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:81:25:81:33 | untrusted | LDAP query parameter is derived from $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | a user-provided value |
43+
| LDAPInjection.go:59:3:59:11 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:59:3:59:11 | untrusted | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
44+
| LDAPInjection.go:61:3:61:51 | ...+... | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:61:3:61:51 | ...+... | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
45+
| LDAPInjection.go:62:3:62:33 | slice literal | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:62:3:62:33 | slice literal | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
46+
| LDAPInjection.go:66:3:66:11 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:66:3:66:11 | untrusted | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
47+
| LDAPInjection.go:68:3:68:51 | ...+... | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:68:3:68:51 | ...+... | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
48+
| LDAPInjection.go:69:3:69:33 | slice literal | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:69:3:69:33 | slice literal | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
49+
| LDAPInjection.go:73:3:73:11 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:73:3:73:11 | untrusted | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
50+
| LDAPInjection.go:75:3:75:51 | ...+... | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:75:3:75:51 | ...+... | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
51+
| LDAPInjection.go:76:3:76:33 | slice literal | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:76:3:76:33 | slice literal | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
52+
| LDAPInjection.go:80:22:80:30 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:80:22:80:30 | untrusted | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |
53+
| LDAPInjection.go:81:25:81:33 | untrusted | LDAPInjection.go:57:15:57:29 | call to UserAgent : string | LDAPInjection.go:81:25:81:33 | untrusted | LDAP query parameter depends on a $@. | LDAPInjection.go:57:15:57:29 | call to UserAgent | user-provided value |

‎go/ql/test/experimental/CWE-840/ConditionalBypass.expected

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
| ConditionalBypassBad.go:9:5:9:46 | ...!=... | This comparison compares user-controlled values from $@ and $@, and hence can be bypassed. | ConditionalBypassBad.go:9:5:9:12 | selection of Header : Header | here | ConditionalBypassBad.go:9:41:9:46 | selection of Host : string | here |
2-
| condition.go:9:5:9:46 | ...!=... | This comparison compares user-controlled values from $@ and $@, and hence can be bypassed. | condition.go:9:5:9:12 | selection of Header : Header | here | condition.go:9:41:9:46 | selection of Host : string | here |
3-
| condition.go:16:5:16:62 | ...!=... | This comparison compares user-controlled values from $@ and $@, and hence can be bypassed. | condition.go:16:5:16:12 | selection of Header : Header | here | condition.go:16:41:16:48 | selection of Header : Header | here |
1+
| ConditionalBypassBad.go:9:5:9:46 | ...!=... | This comparison of a $@ with another $@ can be bypassed by a malicious user. | ConditionalBypassBad.go:9:5:9:12 | selection of Header | user-controlled value | ConditionalBypassBad.go:9:41:9:46 | selection of Host | user-controlled value |
2+
| condition.go:9:5:9:46 | ...!=... | This comparison of a $@ with another $@ can be bypassed by a malicious user. | condition.go:9:5:9:12 | selection of Header | user-controlled value | condition.go:9:41:9:46 | selection of Host | user-controlled value |
3+
| condition.go:16:5:16:62 | ...!=... | This comparison of a $@ with another $@ can be bypassed by a malicious user. | condition.go:16:5:16:12 | selection of Header | user-controlled value | condition.go:16:41:16:48 | selection of Header | user-controlled value |

0 commit comments

Comments
 (0)