Java: Update the alert messages to better follow the style guide #10528
Conversation
erik-krogh
force-pushed
the
java-followMsg
branch
from
369e782 to
7e78434
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Found 16 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Found 15 potential problems in the proposed changes. Check the Files changed tab for more details.
erik-krogh
force-pushed
the
java-followMsg
branch
from
3641279 to
7bca705
Compare
erik-krogh
added
the
depends on internal PR
erik-krogh
force-pushed
the
java-followMsg
branch
from
7bca705 to
27884a6
Compare
There was a problem hiding this comment.
Found 15 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Found 15 potential problems in the proposed changes. Check the Files changed tab for more details.
erik-krogh
force-pushed
the
java-followMsg
branch
from
9c134a7 to
84e7e22
Compare
There was a problem hiding this comment.
Found 15 potential problems in the proposed changes. Check the Files changed tab for more details.
erik-krogh
force-pushed
the
java-followMsg
branch
from
84e7e22 to
614ab31
Compare
There was a problem hiding this comment.
Found 13 potential problems in the proposed changes. Check the Files changed tab for more details.
erik-krogh
force-pushed
the
java-followMsg
branch
from
614ab31 to
da4376c
Compare
There was a problem hiding this comment.
Found 13 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Found 13 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Found 13 potential problems in the proposed changes. Check the Files changed tab for more details.
erik-krogh
force-pushed
the
java-followMsg
branch
from
491b18c to
fd3f896
Compare
There was a problem hiding this comment.
Found 13 potential problems in the proposed changes. Check the Files changed tab for more details.
erik-krogh
force-pushed
the
java-followMsg
branch
from
fd3f896 to
46b5bf3
Compare
There was a problem hiding this comment.
Found 13 potential problems in the proposed changes. Check the Files changed tab for more details.
| from EqualsMethod m | ||
| where | ||
| exists(m.getBody()) and | ||
| exists(Parameter p | p = m.getAParameter() | | ||
| // The parameter has no type test | ||
| not hasTypeTest(p) and | ||
| // If the parameter is passed to a method for which we don't have the source | ||
| // we assume it's ok | ||
| not exists(MethodAccess ma | | ||
| not exists(ma.getMethod().getBody()) and | ||
| ma.getAnArgument() = p.getAnAccess() | ||
| ) | ||
| ) and | ||
| not m.getDeclaringType() instanceof Interface and | ||
| // Exclude `equals` methods that implement reference-equality. | ||
| not m instanceof ReferenceEquals and | ||
| not m instanceof UnimplementedEquals | ||
| select m, "equals() method does not check argument type." | ||
| select m, "This 'equals()' method does not check argument type." |
Check warning
Code scanning / CodeQL
Consistent alert message
| from QueryInjectionSink query, DataFlow::PathNode source, DataFlow::PathNode sink | ||
| where queryTaintedBy(query, source, sink) | ||
| select query, source, sink, "This SQL query depends on $@.", source.getNode(), | ||
| "a user-provided value" | ||
| select query, source, sink, "This query depends on a $@.", source.getNode(), "user-provided value" |
Check warning
Code scanning / CodeQL
Consistent alert message
| from LogInjectionConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink | ||
| where cfg.hasFlowPath(source, sink) | ||
| select source.getNode(), source, sink, "This user-provided value flows to a $@.", sink.getNode(), | ||
| "log entry" | ||
| select sink.getNode(), source, sink, "Log entry depends on a $@.", source.getNode(), | ||
| "user-provided value" |
Check warning
Code scanning / CodeQL
Consistent alert message
| from | ||
| DataFlow::PathNode source, DataFlow::PathNode sink, StringFormat formatCall, | ||
| ExternallyControlledFormatStringConfig conf | ||
| where conf.hasFlowPath(source, sink) and sink.getNode().asExpr() = formatCall.getFormatArgument() | ||
| select formatCall.getFormatArgument(), source, sink, | ||
| "$@ flows to here and is used in a format string.", source.getNode(), "User-provided value" | ||
| select formatCall.getFormatArgument(), source, sink, "Format string depends on a $@.", | ||
| source.getNode(), "user-provided value" |
Check warning
Code scanning / CodeQL
Consistent alert message
| from DataFlow::PathNode source, DataFlow::PathNode sink, UnsafeDeserializationConfig conf | ||
| where conf.hasFlowPath(source, sink) | ||
| select sink.getNode().(UnsafeDeserializationSink).getMethodAccess(), source, sink, | ||
| "Unsafe deserialization of $@.", source.getNode(), "user input" | ||
| "Unsafe deserialization depends on a $@.", source.getNode(), "user-provided value" |
Check warning
Code scanning / CodeQL
Consistent alert message
| from DataFlow::PathNode source, DataFlow::PathNode sink, XxeConfig conf | ||
| where conf.hasFlowPath(source, sink) | ||
| select sink.getNode(), source, sink, | ||
| "A $@ is parsed as XML without guarding against external entity expansion.", source.getNode(), | ||
| "user-provided value" | ||
| "XML parsing depends on a $@ without guarding against external entity expansion.", | ||
| source.getNode(), "user-provided value" |
Check warning
Code scanning / CodeQL
Consistent alert message
| from DataFlow::PathNode source, DataFlow::PathNode sink, PolynomialBackTrackingTerm regexp | ||
| where hasPolynomialReDoSResult(source, sink, regexp) | ||
| select sink, source, sink, | ||
| "This $@ that depends on $@ may run slow on strings " + regexp.getPrefixMessage() + | ||
| "This $@ that depends on a $@ may run slow on strings " + regexp.getPrefixMessage() + | ||
| "with many repetitions of '" + regexp.getPumpString() + "'.", regexp, "regular expression", | ||
| source.getNode(), "a user-provided value" | ||
| source.getNode(), "user-provided value" |
Check warning
Code scanning / CodeQL
Consistent alert message
| from LocalVariableDeclExpr ve, LocalVariableDecl v | ||
| where | ||
| v = ve.getVariable() and | ||
| not assigned(v) and | ||
| not read(v) and | ||
| (not exists(ve.getInit()) or exprHasNoEffect(ve.getInit())) and | ||
| // Remove contexts where Java forces a variable declaration: enhanced-for and catch clauses. | ||
| // Rules about catch clauses belong in an exception handling query | ||
| not exceptionVariable(ve) and | ||
| not enhancedForVariable(ve) | ||
| select v, | ||
| "Unused local variable " + v.getName() + | ||
| ". The variable is never read or written to and should be removed." | ||
| select v, "Variable " + v.getName() + " is not used." |
Check warning
Code scanning / CodeQL
Consistent alert message
| from | ||
| DataFlow::PathNode source, DataFlow::PathNode sink, MethodAccess m, Expr e, | ||
| ConditionalBypassFlowConfig conf | ||
| where | ||
| conditionControlsMethod(m, e) and | ||
| sink.getNode().asExpr() = e and | ||
| conf.hasFlowPath(source, sink) | ||
| select m, source, sink, | ||
| "Sensitive method may not be executed depending on $@, which flows from $@.", e, "this condition", | ||
| source.getNode(), "user input" | ||
| "Sensitive method may not be executed depending on a $@, which flows from $@.", e, | ||
| "this condition", source.getNode(), "user-controlled value" |
Check warning
Code scanning / CodeQL
Consistent alert message
The tests are failing, and that's expected (there is an internal PR that updates the expected output).
I've been working on making the alert-messages consistent across languages, and some fan out work from that is ensuring that the alert-messages follow the style guide (which I've also revised).
Fixes done as part of this PR:
Some of the messages become inconsistent with other languages, but I'm updating one language at a time, so some will get out of sync.
Some other PRs in this series: JS, Py, Rb, Go, C, C# (draft).
Style guide update.
Side-note:
Copilot is great at writing consistent alert-messages. It reads the other tabs I have open.