Skip to content

Commit 9ac797a

Browse files
pavgustpavgust
committed
Merge branch 'master' of git.semmle.com:Semmle/public-ql into imp/csharp
2 parents 591fbb8 + 7077116 commit 9ac797a

File tree

94 files changed

+943
-151
lines changed

Some content is hidden

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

94 files changed

+943
-151
lines changed

‎change-notes/1.18/analysis-javascript.md

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# Improvements to JavaScript analysis
2+
3+
## General improvements
4+
5+
* Additional heuristics have been added to `semmle.javascript.heuristics`. Add `import semmle.javascript.heuristics.all` to a query in order to activate all of the heuristics at once.
6+
7+
* Modelling of data flow through destructuring assignments has been improved. This may give additional results for the security queries and other queries that rely on data flow.
8+
9+
* Support for popular libraries has been improved. Consequently, queries may produce more results on code bases that use the following libraries:
10+
- [browserid-crypto](https://github.com/mozilla/browserid-crypto)
11+
- [cookie-parser](https://github.com/expressjs/cookie-parser)
12+
- [cookie-session](https://github.com/expressjs/cookie-session)
13+
- [crypto-js](https://github.com/https://github.com/brix/crypto-js)
14+
- [express-jwt](https://github.com/auth0/express-jwt)
15+
- [express-session](https://github.com/expressjs/session)
16+
- [forge](https://github.com/digitalbazaar/forge)
17+
- [MySQL2](https://github.com/sidorares/node-mysql2)
18+
19+
## New queries
20+
21+
| **Query** | **Tags** | **Purpose** |
22+
|-----------------------------|-----------|--------------------------------------------------------------------|
23+
| Use of externally-controlled format string (`js/tainted-format-string`) | security, external/cwe/cwe-134 | Highlights format strings containing user-provided data, indicating a violation of [CWE-134](https://cwe.mitre.org/data/definitions/134.html). Results shown on lgtm by default. |
24+
25+
## Changes to existing queries
26+
27+
| **Query** | **Expected impact** | **Change** |
28+
|----------------------------|------------------------|------------------------------------------------------------------|
29+
| Arguments redefined | Fewer results | This rule previously also flagged redefinitions of `eval`. This was an oversight that is now fixed. |
30+
| CORS misconfiguration for credentials transfer | More true-positive results | This rule now treats header names case-insensitively. |
31+
| Hard-coded credentials | More true-positive results | This rule now recognizes secret cryptographic keys. |
32+
| Insecure randomness | More true-positive results | This rule now recognizes secret cryptographic keys. |
33+
| Missing X-Frame-Options HTTP header | Fewer false-positive results | This rule now treats header names case-insensitively. |
34+
| Reflected cross-site scripting | Fewer false-positive results | This rule now treats header names case-insensitively. |
35+
| Server-side URL redirect | More true-positive results | This rule now treats header names case-insensitively. |
36+
| Uncontrolled command line | More true-positive results | This rule now recognizes indirect command injection through `sh -c` and similar. |
37+
| Unused variable | Fewer results | This rule no longer flags class expressions that could be made anonymous. While technically true, these results are not interesting. |
38+
39+
## Changes to QL libraries
40+
41+
* HTTP header names are now always normalized to lower case to reflect the fact that they are case insensitive. In particular, the result of `HeaderDefinition.getAHeaderName`, and the first parameter of `HeaderDefinition.defines`, `ExplicitHeaderDefinition.definesExplicitly` and `RouteHandler.getAResponseHeader` is now always a lower-case string.

‎javascript/config/suites/javascript/security

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
+ semmlecode-javascript-queries/Security/CWE-089/SqlInjection.ql: /Security/CWE/CWE-089
77
+ semmlecode-javascript-queries/Security/CWE-094/CodeInjection.ql: /Security/CWE/CWE-094
88
+ semmlecode-javascript-queries/Security/CWE-116/IncompleteSanitization.ql: /Security/CWE/CWE-116
9+
+ semmlecode-javascript-queries/Security/CWE-134/TaintedFormatString.ql: /Security/CWE/CWE-134
910
+ semmlecode-javascript-queries/Security/CWE-209/StackTraceExposure.ql: /Security/CWE/CWE-209
1011
+ semmlecode-javascript-queries/Security/CWE-312/CleartextStorage.ql: /Security/CWE/CWE-312
1112
+ semmlecode-javascript-queries/Security/CWE-313/PasswordInConfigurationFile.ql: /Security/CWE/CWE-313

‎javascript/config/suites/lgtm/javascript-alerts-lgtm

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# DO NOT EDIT
2+
# This is a stub file. The actual suite of queries to run is generated
3+
# automatically based on query precision and severity.

‎javascript/config/suites/lgtm/javascript-lgtm

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
@import "javascript-util-lgtm"
2+
@import "javascript-alerts-lgtm"
3+
@import "javascript-metrics-lgtm"

‎javascript/config/suites/lgtm/javascript-metrics-lgtm

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
+ semmlecode-javascript-queries/Comments/FCommentedOutCode.ql: /Metrics/Files
2+
@_namespace com.lgtm/javascript-queries
3+
+ semmlecode-javascript-queries/Metrics/FLinesOfCode.ql: /Metrics/Files
4+
@_namespace com.lgtm/javascript-queries
5+
+ semmlecode-javascript-queries/Metrics/FLinesOfComment.ql: /Metrics/Documentation
6+
@_namespace com.lgtm/javascript-queries
7+
+ semmlecode-javascript-queries/Metrics/FLinesOfDuplicatedCode.ql: /Metrics/Duplication
8+
@_namespace com.lgtm/javascript-queries
9+
+ semmlecode-javascript-queries/Metrics/FLinesOfSimilarCode.ql: /Metrics/Duplication
10+
@_namespace com.lgtm/javascript-queries
11+
+ semmlecode-javascript-queries/Metrics/FNumberOfTests.ql: /Metrics/Files
12+
@_namespace com.lgtm/javascript-queries
13+
14+
+ semmlecode-javascript-queries/Metrics/Dependencies/ExternalDependencies.ql
15+
@_namespace com.lgtm/javascript-queries
16+
+ semmlecode-javascript-queries/Metrics/Dependencies/ExternalDependenciesSourceLinks.ql
17+
@_namespace com.lgtm/javascript-queries
18+

‎javascript/config/suites/lgtm/javascript-util-lgtm

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
+ semmlecode-javascript-queries/definitions.ql
2+
@_namespace com.lgtm/javascript-queries
3+
+ semmlecode-javascript-queries/AlertSuppression.ql
4+
@_namespace com.lgtm/javascript-queries
5+
+ semmlecode-javascript-queries/filters/ClassifyFiles.ql
6+
@_namespace com.lgtm/javascript-queries

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import javascript
1616
import semmle.javascript.security.dataflow.CommandInjection::CommandInjection
1717

18-
from Configuration cfg, DataFlow::Node source, DataFlow::Node sink
19-
where cfg.hasFlow(source, sink)
20-
select sink, "This command depends on $@.", source, "a user-provided value"
18+
from Configuration cfg, DataFlow::Node source, DataFlow::Node sink, DataFlow::Node highlight
19+
where cfg.hasFlow(source, sink) and
20+
if cfg.isSink(sink, _) then cfg.isSink(sink, highlight) else highlight = sink
21+
select highlight, "This command depends on $@.", source, "a user-provided value"

‎javascript/ql/src/Security/CWE-451/MissingXFrameOptions.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,5 @@ import javascript
1515
import semmle.javascript.frameworks.HTTP
1616

1717
from HTTP::ServerDefinition server
18-
where not exists(server.getARouteHandler().getAResponseHeader("X-Frame-Options"))
18+
where not exists(server.getARouteHandler().getAResponseHeader("x-frame-options"))
1919
select server, "This server never sets the 'X-Frame-Options' HTTP header."

‎javascript/ql/src/javascript.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import semmle.javascript.dataflow.TypeInference
4848
import semmle.javascript.frameworks.AngularJS
4949
import semmle.javascript.frameworks.AWS
5050
import semmle.javascript.frameworks.Azure
51+
import semmle.javascript.frameworks.Babel
5152
import semmle.javascript.frameworks.Credentials
5253
import semmle.javascript.frameworks.CryptoLibraries
5354
import semmle.javascript.frameworks.DigitalOcean

‎javascript/ql/src/semmle/javascript/Concepts.qll

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ import javascript
1313
abstract class SystemCommandExecution extends DataFlow::Node {
1414
/** Gets an argument to this execution that specifies the command. */
1515
abstract DataFlow::Node getACommandArgument();
16+
17+
/**
18+
* Gets an argument to this command execution that specifies the argument list
19+
* to the command.
20+
*/
21+
DataFlow::Node getArgumentList() { none() }
1622
}
1723

1824
/**

‎javascript/ql/src/semmle/javascript/DataFlow.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,7 @@ private deprecated class IifeParameterFlow extends VarDefFlow {
239239
ImmediatelyInvokedFunctionExpr iife;
240240

241241
IifeParameterFlow() {
242+
this instanceof SimpleParameter and
242243
iife.argumentPassing(this, _)
243244
}
244245

‎javascript/ql/src/semmle/javascript/Expr.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ class Expr extends @expr, ExprOrStmt, ExprOrType, AST::ValueNode {
141141
/**
142142
* Holds if this expression may refer to the initial value of parameter `p`.
143143
*/
144-
predicate mayReferToParameter(SimpleParameter p) {
144+
predicate mayReferToParameter(Parameter p) {
145145
flow().mayReferToParameter(p)
146146
}
147147

@@ -1645,7 +1645,7 @@ class ImmediatelyInvokedFunctionExpr extends Function {
16451645
* conversely, arguments after a spread element do not have a corresponding
16461646
* parameter.
16471647
*/
1648-
predicate argumentPassing(SimpleParameter p, Expr arg) {
1648+
predicate argumentPassing(Parameter p, Expr arg) {
16491649
exists (int parmIdx, int argIdx |
16501650
p = getParameter(parmIdx) and not p.isRestParameter() and
16511651
argIdx = parmIdx + getArgumentOffset() and arg = getArgument(argIdx) and

‎javascript/ql/src/semmle/javascript/Variables.qll

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,8 @@ class VarRef extends @varref, Identifier, BindingPattern, LexicalRef {
293293
/** Gets the variable this identifier refers to. */
294294
Variable getVariable() { none() } // Overriden in VarAccess and VarDecl
295295

296+
override string getName() { result = Identifier.super.getName() }
297+
296298
override VarRef getABindingVarRef() { result = this }
297299

298300
override predicate isImpure() { none() }
@@ -398,6 +400,9 @@ class GlobalVarAccess extends VarAccess {
398400
* the ECMAScript standard for more details.
399401
*/
400402
class BindingPattern extends @pattern, Expr {
403+
/** Gets the name of this binding pattern if it is an identifier. */
404+
string getName() { none() }
405+
401406
/** Gets a variable reference in binding position within this pattern. */
402407
VarRef getABindingVarRef() { none() }
403408

@@ -428,6 +433,8 @@ class BindingPattern extends @pattern, Expr {
428433

429434
/** A destructuring pattern, that is, either an array pattern or an object pattern. */
430435
abstract class DestructuringPattern extends BindingPattern {
436+
/** Gets the rest pattern of this destructuring pattern, if any. */
437+
abstract Expr getRest();
431438
}
432439

433440
/** An identifier that declares a variable. */
@@ -477,7 +484,7 @@ class ArrayPattern extends DestructuringPattern, @arraypattern {
477484
}
478485

479486
/** Gets the rest pattern of this array pattern, if any. */
480-
Expr getRest() {
487+
override Expr getRest() {
481488
result = getChildExpr(-1)
482489
}
483490

@@ -535,7 +542,7 @@ class ObjectPattern extends DestructuringPattern, @objectpattern {
535542
}
536543

537544
/** Gets the rest property pattern of this object pattern, if any. */
538-
Expr getRest() {
545+
override Expr getRest() {
539546
result = getChildExpr(-1)
540547
}
541548

‎javascript/ql/src/semmle/javascript/dataflow/Configuration.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ private predicate callInputStep(Function f, DataFlow::Node invk,
400400
DataFlow::Configuration cfg) {
401401
isRelevant(pred, cfg) and
402402
(
403-
exists (SimpleParameter parm |
403+
exists (Parameter parm |
404404
argumentPassing(invk, pred, f, parm) and
405405
succ = DataFlow::parameterNode(parm)
406406
)

0 commit comments

Comments
 (0)