diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ConditionalBypassQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ConditionalBypassQuery.qll index 9a5e9c45b77c..d6cd673860bf 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ConditionalBypassQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ConditionalBypassQuery.qll @@ -30,3 +30,96 @@ class Configuration extends TaintTracking::Configuration { dst.asExpr().(Comparison).hasOperands(src.asExpr(), any(ConstantExpr c)) } } + +/** + * Holds if the value of `nd` flows into `guard`. + */ +predicate flowsToGuardExpr(DataFlow::Node nd, SensitiveActionGuardConditional guard) { + nd = guard or + flowsToGuardExpr(nd.getASuccessor(), guard) +} + +/** + * A comparison that guards a sensitive action, e.g. the comparison in: + * `var ok = x == y; if (ok) login()`. + */ +class SensitiveActionGuardComparison extends Comparison { + SensitiveActionGuardConditional guard; + + SensitiveActionGuardComparison() { flowsToGuardExpr(DataFlow::valueNode(this), guard) } + + /** + * Gets the guard that uses this comparison. + */ + SensitiveActionGuardConditional getGuard() { result = guard } +} + +/** + * An intermediary sink to enable reuse of the taint configuration. + * This sink should not be presented to the client of this query. + */ +class SensitiveActionGuardComparisonOperand extends Sink { + SensitiveActionGuardComparison comparison; + + SensitiveActionGuardComparisonOperand() { asExpr() = comparison.getAnOperand() } + + override SensitiveAction getAction() { result = comparison.getGuard().getAction() } +} + +/** + * Holds if `sink` guards `action`, and `source` taints `sink`. + * + * If flow from `source` taints `sink`, then an attacker can + * control if `action` should be executed or not. + */ +predicate isTaintedGuardForSensitiveAction( + DataFlow::PathNode sink, DataFlow::PathNode source, SensitiveAction action +) { + action = sink.getNode().(Sink).getAction() and + // exclude the intermediary sink + not sink.getNode() instanceof SensitiveActionGuardComparisonOperand and + exists(Configuration cfg | + // ordinary taint tracking to a guard + cfg.hasFlowPath(source, sink) + or + // taint tracking to both operands of a guard comparison + exists( + SensitiveActionGuardComparison cmp, DataFlow::PathNode lSource, DataFlow::PathNode rSource, + DataFlow::PathNode lSink, DataFlow::PathNode rSink + | + sink.getNode() = cmp.getGuard() and + cfg.hasFlowPath(lSource, lSink) and + lSink.getNode() = DataFlow::valueNode(cmp.getLeftOperand()) and + cfg.hasFlowPath(rSource, rSink) and + rSink.getNode() = DataFlow::valueNode(cmp.getRightOperand()) + | + source = lSource or + source = rSource + ) + ) +} + +/** + * Holds if `e` effectively guards access to `action` by returning or throwing early. + * + * Example: `if (e) return; action(x)`. + */ +predicate isEarlyAbortGuard(DataFlow::PathNode e, SensitiveAction action) { + exists(IfStmt guard | + // `e` is in the condition of an if-statement ... + e.getNode().(Sink).asExpr().getParentExpr*() = guard.getCondition() and + // ... where the then-branch always throws or returns + exists(Stmt abort | + abort instanceof ThrowStmt or + abort instanceof ReturnStmt + | + abort.nestedIn(guard) and + abort.getBasicBlock().(ReachableBasicBlock).postDominates(guard.getThen().getBasicBlock()) + ) and + // ... and the else-branch does not exist + not exists(guard.getElse()) + | + // ... and `action` is outside the if-statement + not action.asExpr().getEnclosingStmt().nestedIn(guard) + ) +} diff --git a/javascript/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.inc.qhelp b/javascript/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.inc.qhelp new file mode 100644 index 000000000000..226eb9ac77fb --- /dev/null +++ b/javascript/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.inc.qhelp @@ -0,0 +1,57 @@ + + + +

Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports +external APIs that use untrusted data. The results are not filtered so that you can audit all examples. The query provides data for security reviews of the application and you can also use it to identify external APIs that should be modeled as either taint steps, or sinks for specific problems.

+ +

An external API is defined as a method call to a method that is not defined in the source code, not overridden +in the source code, and is not modeled as a taint step in the default taint library. External APIs may be from the +third-party dependencies or from internal dependencies. The query reports uses of +untrusted data one of the arguments of external API call or in the return value from a callback passed to an external API.

+ +
+ + +

For each result:

+ + + +

Otherwise, the result is likely uninteresting. Custom versions of this query can extend the SafeExternalAPIMethod +class to exclude known safe external APIs from future analysis.

+ +
+ + +

In this first example, a query parameter is read from the req parameter and then ultimately used in a call to the +res.send external API:

+ + + +

This is a reflected XSS sink. The XSS query should therefore be reviewed to confirm that this sink is appropriately modeled, +and if it is, to confirm that the query reports this particular result, or that the result is a false positive due to +some existing sanitization.

+ +

In this second example, again a query parameter is read from req.

+ + + +

If the query reported the call to path.join on line 4, this would suggest that this external API is +not currently modeled as a taint step in the taint tracking library. The next step would be to model this as a taint step, then +re-run the query to determine what additional results might be found. In this example, it seems the result of the +path.join will be used as a file path, leading to a path traversal vulnerability.

+ +

Note that both examples are correctly handled by the standard taint tracking library and security queries.

+
+ + + +
diff --git a/javascript/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.qhelp b/javascript/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.qhelp index 226eb9ac77fb..ffe93665fda5 100644 --- a/javascript/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.qhelp +++ b/javascript/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.qhelp @@ -1,57 +1,6 @@ +"-//Semmle//qhelp//EN" +"qhelp.dtd"> - -

Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports -external APIs that use untrusted data. The results are not filtered so that you can audit all examples. The query provides data for security reviews of the application and you can also use it to identify external APIs that should be modeled as either taint steps, or sinks for specific problems.

- -

An external API is defined as a method call to a method that is not defined in the source code, not overridden -in the source code, and is not modeled as a taint step in the default taint library. External APIs may be from the -third-party dependencies or from internal dependencies. The query reports uses of -untrusted data one of the arguments of external API call or in the return value from a callback passed to an external API.

- -
- - -

For each result:

- - - -

Otherwise, the result is likely uninteresting. Custom versions of this query can extend the SafeExternalAPIMethod -class to exclude known safe external APIs from future analysis.

- -
- - -

In this first example, a query parameter is read from the req parameter and then ultimately used in a call to the -res.send external API:

- - - -

This is a reflected XSS sink. The XSS query should therefore be reviewed to confirm that this sink is appropriately modeled, -and if it is, to confirm that the query reports this particular result, or that the result is a false positive due to -some existing sanitization.

- -

In this second example, again a query parameter is read from req.

- - - -

If the query reported the call to path.join on line 4, this would suggest that this external API is -not currently modeled as a taint step in the taint tracking library. The next step would be to model this as a taint step, then -re-run the query to determine what additional results might be found. In this example, it seems the result of the -path.join will be used as a file path, leading to a path traversal vulnerability.

- -

Note that both examples are correctly handled by the standard taint tracking library and security queries.

-
- - - -
+ + \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-078/CommandInjection.inc.qhelp b/javascript/ql/src/Security/CWE-078/CommandInjection.inc.qhelp new file mode 100644 index 000000000000..b7fbc9493fc2 --- /dev/null +++ b/javascript/ql/src/Security/CWE-078/CommandInjection.inc.qhelp @@ -0,0 +1,44 @@ + + + +

Code that passes user input directly to +require('child_process').exec, or some other library +routine that executes a command, allows the user to execute malicious +code.

+ +
+ + +

If possible, use hard-coded string literals to specify the command to run +or library to load. Instead of passing the user input directly to the +process or library function, examine the user input and then choose +among hard-coded string literals.

+ +

If the applicable libraries or commands cannot be determined at +compile time, then add code to verify that the user input string is +safe before using it.

+ +
+ + +

The following example shows code that takes a shell script that can be changed +maliciously by a user, and passes it straight to child_process.exec +without examining it first.

+ + + +
+ + +
  • +OWASP: +Command Injection. +
  • + + + +
    +
    diff --git a/javascript/ql/src/Security/CWE-078/CommandInjection.qhelp b/javascript/ql/src/Security/CWE-078/CommandInjection.qhelp index b7fbc9493fc2..301f803eff72 100644 --- a/javascript/ql/src/Security/CWE-078/CommandInjection.qhelp +++ b/javascript/ql/src/Security/CWE-078/CommandInjection.qhelp @@ -1,44 +1,6 @@ +"-//Semmle//qhelp//EN" +"qhelp.dtd"> - -

    Code that passes user input directly to -require('child_process').exec, or some other library -routine that executes a command, allows the user to execute malicious -code.

    - -
    - - -

    If possible, use hard-coded string literals to specify the command to run -or library to load. Instead of passing the user input directly to the -process or library function, examine the user input and then choose -among hard-coded string literals.

    - -

    If the applicable libraries or commands cannot be determined at -compile time, then add code to verify that the user input string is -safe before using it.

    - -
    - - -

    The following example shows code that takes a shell script that can be changed -maliciously by a user, and passes it straight to child_process.exec -without examining it first.

    - - - -
    - - -
  • -OWASP: -Command Injection. -
  • - - - -
    -
    + + \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-079/Xss.inc.qhelp b/javascript/ql/src/Security/CWE-079/Xss.inc.qhelp new file mode 100644 index 000000000000..c974c87b1882 --- /dev/null +++ b/javascript/ql/src/Security/CWE-079/Xss.inc.qhelp @@ -0,0 +1,57 @@ + + + + +

    +Directly writing user input (for example, a URL query parameter) to a webpage +without properly sanitizing the input first, allows for a cross-site scripting vulnerability. +

    +

    +This kind of vulnerability is also called DOM-based cross-site scripting, to distinguish +it from other types of cross-site scripting. +

    +
    + + +

    +To guard against cross-site scripting, consider using contextual output encoding/escaping before +writing user input to the page, or one of the other solutions that are mentioned in the +references. +

    +
    + + +

    +The following example shows part of the page URL being written directly to the document, +leaving the website vulnerable to cross-site scripting. +

    + +
    + + +
  • +OWASP: +DOM based +XSS Prevention Cheat Sheet. +
  • +
  • +OWASP: +XSS +(Cross Site Scripting) Prevention Cheat Sheet. +
  • +
  • +OWASP +DOM Based XSS. +
  • +
  • +OWASP +Types of Cross-Site +Scripting. +
  • +
  • +Wikipedia: Cross-site scripting. +
  • +
    +
    diff --git a/javascript/ql/src/Security/CWE-079/Xss.qhelp b/javascript/ql/src/Security/CWE-079/Xss.qhelp index c974c87b1882..8dcc31d618c9 100644 --- a/javascript/ql/src/Security/CWE-079/Xss.qhelp +++ b/javascript/ql/src/Security/CWE-079/Xss.qhelp @@ -1,57 +1,6 @@ +"-//Semmle//qhelp//EN" +"qhelp.dtd"> - - -

    -Directly writing user input (for example, a URL query parameter) to a webpage -without properly sanitizing the input first, allows for a cross-site scripting vulnerability. -

    -

    -This kind of vulnerability is also called DOM-based cross-site scripting, to distinguish -it from other types of cross-site scripting. -

    -
    - - -

    -To guard against cross-site scripting, consider using contextual output encoding/escaping before -writing user input to the page, or one of the other solutions that are mentioned in the -references. -

    -
    - - -

    -The following example shows part of the page URL being written directly to the document, -leaving the website vulnerable to cross-site scripting. -

    - -
    - - -
  • -OWASP: -DOM based -XSS Prevention Cheat Sheet. -
  • -
  • -OWASP: -XSS -(Cross Site Scripting) Prevention Cheat Sheet. -
  • -
  • -OWASP -DOM Based XSS. -
  • -
  • -OWASP -Types of Cross-Site -Scripting. -
  • -
  • -Wikipedia: Cross-site scripting. -
  • -
    -
    + + \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-089/SqlInjection.inc.qhelp b/javascript/ql/src/Security/CWE-089/SqlInjection.inc.qhelp new file mode 100644 index 000000000000..8cdc2419d47a --- /dev/null +++ b/javascript/ql/src/Security/CWE-089/SqlInjection.inc.qhelp @@ -0,0 +1,62 @@ + + + + +

    +If a database query (such as a SQL or NoSQL query) is built from +user-provided data without sufficient sanitization, a malicious user +may be able to run malicious database queries. +

    +
    + + +

    +Most database connector libraries offer a way of safely +embedding untrusted data into a query by means of query parameters +or prepared statements. +

    +

    +For NoSQL queries, make use of an operator like MongoDB's $eq +to ensure that untrusted data is interpreted as a literal value and not as +a query object. +

    +
    + + +

    +In the following example, assume the function handler is +an HTTP request handler in a web application, whose parameter +req contains the request object. +

    + +

    +The handler constructs two copies of the same SQL query involving +user input taken from the request object, once unsafely using +string concatenation, and once safely using query parameters. +

    + +

    +In the first case, the query string query1 is built by +directly concatenating a user-supplied request parameter with some +string literals. The parameter may include quote characters, so this +code is vulnerable to a SQL injection attack. +

    + +

    +In the second case, the parameter is embedded into the query string +query2 using query parameters. In this example, we use +the API offered by the pg Postgres database connector +library, but other libraries offer similar features. This version is +immune to injection attacks. +

    + + +
    + + +
  • Wikipedia: SQL injection.
  • +
  • MongoDB: $eq operator.
  • +
    +
    diff --git a/javascript/ql/src/Security/CWE-089/SqlInjection.qhelp b/javascript/ql/src/Security/CWE-089/SqlInjection.qhelp index 8cdc2419d47a..cdbc98667df5 100644 --- a/javascript/ql/src/Security/CWE-089/SqlInjection.qhelp +++ b/javascript/ql/src/Security/CWE-089/SqlInjection.qhelp @@ -1,62 +1,6 @@ +"-//Semmle//qhelp//EN" +"qhelp.dtd"> - - -

    -If a database query (such as a SQL or NoSQL query) is built from -user-provided data without sufficient sanitization, a malicious user -may be able to run malicious database queries. -

    -
    - - -

    -Most database connector libraries offer a way of safely -embedding untrusted data into a query by means of query parameters -or prepared statements. -

    -

    -For NoSQL queries, make use of an operator like MongoDB's $eq -to ensure that untrusted data is interpreted as a literal value and not as -a query object. -

    -
    - - -

    -In the following example, assume the function handler is -an HTTP request handler in a web application, whose parameter -req contains the request object. -

    - -

    -The handler constructs two copies of the same SQL query involving -user input taken from the request object, once unsafely using -string concatenation, and once safely using query parameters. -

    - -

    -In the first case, the query string query1 is built by -directly concatenating a user-supplied request parameter with some -string literals. The parameter may include quote characters, so this -code is vulnerable to a SQL injection attack. -

    - -

    -In the second case, the parameter is embedded into the query string -query2 using query parameters. In this example, we use -the API offered by the pg Postgres database connector -library, but other libraries offer similar features. This version is -immune to injection attacks. -

    - - -
    - - -
  • Wikipedia: SQL injection.
  • -
  • MongoDB: $eq operator.
  • -
    -
    + + \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-094/CodeInjection.inc.qhelp b/javascript/ql/src/Security/CWE-094/CodeInjection.inc.qhelp new file mode 100644 index 000000000000..7f06ac78f94f --- /dev/null +++ b/javascript/ql/src/Security/CWE-094/CodeInjection.inc.qhelp @@ -0,0 +1,63 @@ + + + + +

    +Directly evaluating user input (for example, an HTTP request parameter) as code without properly +sanitizing the input first allows an attacker arbitrary code execution. This can occur when user +input is treated as JavaScript, or passed to a framework which interprets it as an expression to be +evaluated. Examples include AngularJS expressions or JQuery selectors. +

    +
    + + +

    +Avoid including user input in any expression which may be dynamically evaluated. If user input must +be included, use context-specific escaping before +including it. It is important that the correct escaping is used for the type of evaluation that will +occur. +

    +
    + + +

    +The following example shows part of the page URL being evaluated as JavaScript code. This allows an +attacker to provide JavaScript within the URL. If an attacker can persuade a user to click on a link +to such a URL, the attacker can evaluate arbitrary JavaScript in the browser of the user to, +for example, steal cookies containing session information. +

    + + + +

    +The following example shows a Pug template being constructed from user input, allowing attackers to run +arbitrary code via a payload such as #{global.process.exit(1)}. +

    + + + +

    +Below is an example of how to use a template engine without any risk of template injection. +The user input is included via an interpolation expression #{username} whose value is provided +as an option to the template, instead of being part of the template string itself: +

    + + +
    + + +
  • +OWASP: +Code Injection. +
  • +
  • +Wikipedia: Code Injection. +
  • +
  • +PortSwigger Research Blog: +Server-Side Template Injection. +
  • +
    +
    diff --git a/javascript/ql/src/Security/CWE-094/CodeInjection.qhelp b/javascript/ql/src/Security/CWE-094/CodeInjection.qhelp index 7f06ac78f94f..784368354731 100644 --- a/javascript/ql/src/Security/CWE-094/CodeInjection.qhelp +++ b/javascript/ql/src/Security/CWE-094/CodeInjection.qhelp @@ -1,63 +1,6 @@ +"-//Semmle//qhelp//EN" +"qhelp.dtd"> - - -

    -Directly evaluating user input (for example, an HTTP request parameter) as code without properly -sanitizing the input first allows an attacker arbitrary code execution. This can occur when user -input is treated as JavaScript, or passed to a framework which interprets it as an expression to be -evaluated. Examples include AngularJS expressions or JQuery selectors. -

    -
    - - -

    -Avoid including user input in any expression which may be dynamically evaluated. If user input must -be included, use context-specific escaping before -including it. It is important that the correct escaping is used for the type of evaluation that will -occur. -

    -
    - - -

    -The following example shows part of the page URL being evaluated as JavaScript code. This allows an -attacker to provide JavaScript within the URL. If an attacker can persuade a user to click on a link -to such a URL, the attacker can evaluate arbitrary JavaScript in the browser of the user to, -for example, steal cookies containing session information. -

    - - - -

    -The following example shows a Pug template being constructed from user input, allowing attackers to run -arbitrary code via a payload such as #{global.process.exit(1)}. -

    - - - -

    -Below is an example of how to use a template engine without any risk of template injection. -The user input is included via an interpolation expression #{username} whose value is provided -as an option to the template, instead of being part of the template string itself: -

    - - -
    - - -
  • -OWASP: -Code Injection. -
  • -
  • -Wikipedia: Code Injection. -
  • -
  • -PortSwigger Research Blog: -Server-Side Template Injection. -
  • -
    -
    + + \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-117/LogInjection.inc.qhelp b/javascript/ql/src/Security/CWE-117/LogInjection.inc.qhelp new file mode 100644 index 000000000000..04a1a3e36e81 --- /dev/null +++ b/javascript/ql/src/Security/CWE-117/LogInjection.inc.qhelp @@ -0,0 +1,47 @@ + + + + + +

    If unsanitized user input is written to a log entry, a malicious user may be able to forge new log entries.

    + +

    Forgery can occur if a user provides some input with characters that are interpreted +when the log output is displayed. If the log is displayed as a plain text file, then new +line characters can be used by a malicious user. If the log is displayed as HTML, then +arbitrary HTML may be included to spoof log entries.

    +
    + + +

    +User input should be suitably sanitized before it is logged. +

    +

    +If the log entries are in plain text then line breaks should be removed from user input, using +String.prototype.replace or similar. Care should also be taken that user input is clearly marked +in log entries. +

    +

    +For log entries that will be displayed in HTML, user input should be HTML-encoded before being logged, to prevent forgery and +other forms of HTML injection. +

    + +
    + + +

    In the first example, a username, provided by the user, is logged using `console.info`. In +the first case, it is logged without any sanitization. In the second case, the username is used to build an error that is logged using `console.error`. +If a malicious user provides `username=Guest%0a[INFO]+User:+Admin%0a` as a username parameter, +the log entry will be splitted in two different lines, where the second line will be `[INFO]+User:+Admin`. +

    + + +

    In the second example, String.prototype.replace is used to ensure no line endings are present in the user input.

    + +
    + + +
  • OWASP: Log Injection.
  • +
    +
    diff --git a/javascript/ql/src/Security/CWE-117/LogInjection.qhelp b/javascript/ql/src/Security/CWE-117/LogInjection.qhelp index 04a1a3e36e81..35581ef14850 100644 --- a/javascript/ql/src/Security/CWE-117/LogInjection.qhelp +++ b/javascript/ql/src/Security/CWE-117/LogInjection.qhelp @@ -1,47 +1,6 @@ +"-//Semmle//qhelp//EN" +"qhelp.dtd"> - - - -

    If unsanitized user input is written to a log entry, a malicious user may be able to forge new log entries.

    - -

    Forgery can occur if a user provides some input with characters that are interpreted -when the log output is displayed. If the log is displayed as a plain text file, then new -line characters can be used by a malicious user. If the log is displayed as HTML, then -arbitrary HTML may be included to spoof log entries.

    -
    - - -

    -User input should be suitably sanitized before it is logged. -

    -

    -If the log entries are in plain text then line breaks should be removed from user input, using -String.prototype.replace or similar. Care should also be taken that user input is clearly marked -in log entries. -

    -

    -For log entries that will be displayed in HTML, user input should be HTML-encoded before being logged, to prevent forgery and -other forms of HTML injection. -

    - -
    - - -

    In the first example, a username, provided by the user, is logged using `console.info`. In -the first case, it is logged without any sanitization. In the second case, the username is used to build an error that is logged using `console.error`. -If a malicious user provides `username=Guest%0a[INFO]+User:+Admin%0a` as a username parameter, -the log entry will be splitted in two different lines, where the second line will be `[INFO]+User:+Admin`. -

    - - -

    In the second example, String.prototype.replace is used to ensure no line endings are present in the user input.

    - -
    - - -
  • OWASP: Log Injection.
  • -
    -
    + + \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-134/TaintedFormatString.inc.qhelp b/javascript/ql/src/Security/CWE-134/TaintedFormatString.inc.qhelp new file mode 100644 index 000000000000..75862895e88f --- /dev/null +++ b/javascript/ql/src/Security/CWE-134/TaintedFormatString.inc.qhelp @@ -0,0 +1,46 @@ + + + + +

    +Functions like the Node.js standard library function util.format accept a +format string that is used to format the remaining arguments by providing inline format +specifiers. If the format string contains unsanitized input from an untrusted source, +then that string may contain unexpected format specifiers that cause garbled output. +

    +
    + + +

    +Either sanitize the input before including it in the format string, or use a +%s specifier in the format string, and pass the untrusted data as corresponding +argument. +

    +
    + + +

    +The following program snippet logs information about an unauthorized access attempt. The +log message includes the user name, and the user's IP address is passed as an additional +argument to console.log to be appended to the message: +

    + +

    +However, if a malicious user provides %d as their user name, console.log +will instead attempt to format the ip argument as a number. Since IP addresses are +not valid numbers, the result of this conversion is NaN. The resulting log message +will read "Unauthorized access attempt by NaN", missing all the information that it was trying to +log in the first place. +

    +

    +Instead, the user name should be included using the %s specifier: +

    + +
    + + +
  • Node.js Documentation: util.format.
  • +
    +
    diff --git a/javascript/ql/src/Security/CWE-134/TaintedFormatString.qhelp b/javascript/ql/src/Security/CWE-134/TaintedFormatString.qhelp index 75862895e88f..8ef0d4f9172d 100644 --- a/javascript/ql/src/Security/CWE-134/TaintedFormatString.qhelp +++ b/javascript/ql/src/Security/CWE-134/TaintedFormatString.qhelp @@ -1,46 +1,6 @@ +"-//Semmle//qhelp//EN" +"qhelp.dtd"> - - -

    -Functions like the Node.js standard library function util.format accept a -format string that is used to format the remaining arguments by providing inline format -specifiers. If the format string contains unsanitized input from an untrusted source, -then that string may contain unexpected format specifiers that cause garbled output. -

    -
    - - -

    -Either sanitize the input before including it in the format string, or use a -%s specifier in the format string, and pass the untrusted data as corresponding -argument. -

    -
    - - -

    -The following program snippet logs information about an unauthorized access attempt. The -log message includes the user name, and the user's IP address is passed as an additional -argument to console.log to be appended to the message: -

    - -

    -However, if a malicious user provides %d as their user name, console.log -will instead attempt to format the ip argument as a number. Since IP addresses are -not valid numbers, the result of this conversion is NaN. The resulting log message -will read "Unauthorized access attempt by NaN", missing all the information that it was trying to -log in the first place. -

    -

    -Instead, the user name should be included using the %s specifier: -

    - -
    - - -
  • Node.js Documentation: util.format.
  • -
    -
    + + \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-346/CorsMisconfigurationForCredentials.inc.qhelp b/javascript/ql/src/Security/CWE-346/CorsMisconfigurationForCredentials.inc.qhelp new file mode 100644 index 000000000000..94aa6cf765f2 --- /dev/null +++ b/javascript/ql/src/Security/CWE-346/CorsMisconfigurationForCredentials.inc.qhelp @@ -0,0 +1,85 @@ + + + + +

    + + A server can send the + "Access-Control-Allow-Credentials" CORS header to control + when a browser may send user credentials in Cross-Origin HTTP + requests. + +

    +

    + + When the Access-Control-Allow-Credentials header + is "true", the Access-Control-Allow-Origin + header must have a value different from "*" in order to + make browsers accept the header. Therefore, to allow multiple origins + for Cross-Origin requests with credentials, the server must + dynamically compute the value of the + "Access-Control-Allow-Origin" header. Computing this + header value from information in the request to the server can + therefore potentially allow an attacker to control the origins that + the browser sends credentials to. + +

    + + + +
    + + +

    + + When the Access-Control-Allow-Credentials header + value is "true", a dynamic computation of the + Access-Control-Allow-Origin header must involve + sanitization if it relies on user-controlled input. + + +

    +

    + + Since the "null" origin is easy to obtain for an + attacker, it is never safe to use "null" as the value of + the Access-Control-Allow-Origin header when the + Access-Control-Allow-Credentials header value is + "true". + +

    +
    + + +

    + + In the example below, the server allows the browser to send + user credentials in a Cross-Origin request. The request header + origins controls the allowed origins for such a + Cross-Origin request. + +

    + + + +

    + + This is not secure, since an attacker can choose the value of + the origin request header to make the browser send + credentials to their own server. The use of a whitelist containing + allowed origins for the Cross-Origin request fixes the issue: + +

    + + +
    + + +
  • Mozilla Developer Network: CORS, Access-Control-Allow-Origin.
  • +
  • Mozilla Developer Network: CORS, Access-Control-Allow-Credentials.
  • +
  • PortSwigger: Exploiting CORS Misconfigurations for Bitcoins and Bounties
  • +
  • W3C: CORS for developers, Advice for Resource Owners
  • +
    +
    diff --git a/javascript/ql/src/Security/CWE-346/CorsMisconfigurationForCredentials.qhelp b/javascript/ql/src/Security/CWE-346/CorsMisconfigurationForCredentials.qhelp index 94aa6cf765f2..b7f473153c42 100644 --- a/javascript/ql/src/Security/CWE-346/CorsMisconfigurationForCredentials.qhelp +++ b/javascript/ql/src/Security/CWE-346/CorsMisconfigurationForCredentials.qhelp @@ -2,84 +2,5 @@ "-//Semmle//qhelp//EN" "qhelp.dtd"> - - -

    - - A server can send the - "Access-Control-Allow-Credentials" CORS header to control - when a browser may send user credentials in Cross-Origin HTTP - requests. - -

    -

    - - When the Access-Control-Allow-Credentials header - is "true", the Access-Control-Allow-Origin - header must have a value different from "*" in order to - make browsers accept the header. Therefore, to allow multiple origins - for Cross-Origin requests with credentials, the server must - dynamically compute the value of the - "Access-Control-Allow-Origin" header. Computing this - header value from information in the request to the server can - therefore potentially allow an attacker to control the origins that - the browser sends credentials to. - -

    - - - -
    - - -

    - - When the Access-Control-Allow-Credentials header - value is "true", a dynamic computation of the - Access-Control-Allow-Origin header must involve - sanitization if it relies on user-controlled input. - - -

    -

    - - Since the "null" origin is easy to obtain for an - attacker, it is never safe to use "null" as the value of - the Access-Control-Allow-Origin header when the - Access-Control-Allow-Credentials header value is - "true". - -

    -
    - - -

    - - In the example below, the server allows the browser to send - user credentials in a Cross-Origin request. The request header - origins controls the allowed origins for such a - Cross-Origin request. - -

    - - - -

    - - This is not secure, since an attacker can choose the value of - the origin request header to make the browser send - credentials to their own server. The use of a whitelist containing - allowed origins for the Cross-Origin request fixes the issue: - -

    - - -
    - - -
  • Mozilla Developer Network: CORS, Access-Control-Allow-Origin.
  • -
  • Mozilla Developer Network: CORS, Access-Control-Allow-Credentials.
  • -
  • PortSwigger: Exploiting CORS Misconfigurations for Bitcoins and Bounties
  • -
  • W3C: CORS for developers, Advice for Resource Owners
  • -
    -
    + + \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-400/RemotePropertyInjection.inc.qhelp b/javascript/ql/src/Security/CWE-400/RemotePropertyInjection.inc.qhelp new file mode 100644 index 000000000000..f1c1f194e863 --- /dev/null +++ b/javascript/ql/src/Security/CWE-400/RemotePropertyInjection.inc.qhelp @@ -0,0 +1,87 @@ + + + + +

    + Dynamically computing object property names from untrusted input + may have multiple undesired consequences. For example, + if the property access is used as part of a write, an + attacker may overwrite vital properties of objects, such as + __proto__. This attack is known as prototype + pollution attack and may serve as a vehicle for denial-of-service + attacks. A similar attack vector, is to replace the + toString property of an object with a primitive. + Whenever toString is then called on that object, either + explicitly or implicitly as part of a type coercion, an exception + will be raised. +

    + +

    + Moreover, if the name of an HTTP header is user-controlled, + an attacker may exploit this to overwrite security-critical headers + such as Access-Control-Allow-Origin or + Content-Security-Policy. +

    +
    + + +

    + The most common case in which prototype pollution vulnerabilities arise + is when JavaScript objects are used for implementing map data + structures. This case should be avoided whenever possible by using the + ECMAScript 2015 Map instead. When this is not possible, an + alternative fix is to prepend untrusted input with a marker character + such as $, before using it in properties accesses. In this way, + the attacker does not have access to built-in properties which do not + start with the chosen character. +

    +

    + When using user input as part of a header name, a sanitization + step should be performed on the input to ensure that the name does not + clash with existing header names such as + Content-Security-Policy. +

    +
    + + +

    + In the example below, the dynamically computed property + prop is accessed on myObj using a + user-controlled value. +

    + + + +

    + This is not secure since an attacker may exploit this code to + overwrite the property __proto__ with an empty function. + If this happens, the concatenation in the console.log + argument will fail with a confusing message such as + "Function.prototype.toString is not generic". If the application does + not properly handle this error, this scenario may result in a serious + denial-of-service attack. The fix is to prepend the user-controlled + string with a marker character such as $ which will + prevent arbitrary property names from being overwritten. +

    + + +
    + + +
  • Prototype pollution attacks: + electron, + lodash, + hoek. +
  • +
  • Penetration testing report: + + header name injection attack +
  • +
  • npm blog post: + + dangers of square bracket notation +
  • +
    +
    diff --git a/javascript/ql/src/Security/CWE-400/RemotePropertyInjection.qhelp b/javascript/ql/src/Security/CWE-400/RemotePropertyInjection.qhelp index f1c1f194e863..3764387608a1 100644 --- a/javascript/ql/src/Security/CWE-400/RemotePropertyInjection.qhelp +++ b/javascript/ql/src/Security/CWE-400/RemotePropertyInjection.qhelp @@ -2,86 +2,5 @@ "-//Semmle//qhelp//EN" "qhelp.dtd"> - - -

    - Dynamically computing object property names from untrusted input - may have multiple undesired consequences. For example, - if the property access is used as part of a write, an - attacker may overwrite vital properties of objects, such as - __proto__. This attack is known as prototype - pollution attack and may serve as a vehicle for denial-of-service - attacks. A similar attack vector, is to replace the - toString property of an object with a primitive. - Whenever toString is then called on that object, either - explicitly or implicitly as part of a type coercion, an exception - will be raised. -

    - -

    - Moreover, if the name of an HTTP header is user-controlled, - an attacker may exploit this to overwrite security-critical headers - such as Access-Control-Allow-Origin or - Content-Security-Policy. -

    -
    - - -

    - The most common case in which prototype pollution vulnerabilities arise - is when JavaScript objects are used for implementing map data - structures. This case should be avoided whenever possible by using the - ECMAScript 2015 Map instead. When this is not possible, an - alternative fix is to prepend untrusted input with a marker character - such as $, before using it in properties accesses. In this way, - the attacker does not have access to built-in properties which do not - start with the chosen character. -

    -

    - When using user input as part of a header name, a sanitization - step should be performed on the input to ensure that the name does not - clash with existing header names such as - Content-Security-Policy. -

    -
    - - -

    - In the example below, the dynamically computed property - prop is accessed on myObj using a - user-controlled value. -

    - - - -

    - This is not secure since an attacker may exploit this code to - overwrite the property __proto__ with an empty function. - If this happens, the concatenation in the console.log - argument will fail with a confusing message such as - "Function.prototype.toString is not generic". If the application does - not properly handle this error, this scenario may result in a serious - denial-of-service attack. The fix is to prepend the user-controlled - string with a marker character such as $ which will - prevent arbitrary property names from being overwritten. -

    - - -
    - - -
  • Prototype pollution attacks: - electron, - lodash, - hoek. -
  • -
  • Penetration testing report: - - header name injection attack -
  • -
  • npm blog post: - - dangers of square bracket notation -
  • -
    -
    + + \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-502/UnsafeDeserialization.inc.qhelp b/javascript/ql/src/Security/CWE-502/UnsafeDeserialization.inc.qhelp new file mode 100644 index 000000000000..5f5b77cbd7de --- /dev/null +++ b/javascript/ql/src/Security/CWE-502/UnsafeDeserialization.inc.qhelp @@ -0,0 +1,52 @@ + + + + +

    +Deserializing untrusted data using any deserialization framework that +allows the construction of arbitrary functions is easily exploitable +and, in many cases, allows an attacker to execute arbitrary code. +

    +
    + + +

    +Avoid deserialization of untrusted data if at all possible. If the +architecture permits it, then use formats like JSON or XML that cannot +represent functions. When using YAML or other formats that support the +serialization and deserialization of functions, ensure that the parser +is configured to disable deserialization of arbitrary functions. +

    +
    + + +

    +The following example calls the load function of the popular +js-yaml package on data that comes from an HTTP request and +hence is inherently unsafe. +

    + +

    +Using the safeLoad function instead (which does not deserialize +YAML-encoded functions) removes the vulnerability. +

    + +
    + + + +
  • +OWASP vulnerability description: +Deserialization of untrusted data. +
  • +
  • +OWASP guidance on deserializing objects: +Deserialization Cheat Sheet. +
  • +
  • +Neal Poole: +Code Execution via YAML in JS-YAML Node.js Module. +
  • +
    + +
    diff --git a/javascript/ql/src/Security/CWE-502/UnsafeDeserialization.qhelp b/javascript/ql/src/Security/CWE-502/UnsafeDeserialization.qhelp index 5f5b77cbd7de..0ff7b27f9e1e 100644 --- a/javascript/ql/src/Security/CWE-502/UnsafeDeserialization.qhelp +++ b/javascript/ql/src/Security/CWE-502/UnsafeDeserialization.qhelp @@ -1,52 +1,6 @@ - + - - -

    -Deserializing untrusted data using any deserialization framework that -allows the construction of arbitrary functions is easily exploitable -and, in many cases, allows an attacker to execute arbitrary code. -

    -
    - - -

    -Avoid deserialization of untrusted data if at all possible. If the -architecture permits it, then use formats like JSON or XML that cannot -represent functions. When using YAML or other formats that support the -serialization and deserialization of functions, ensure that the parser -is configured to disable deserialization of arbitrary functions. -

    -
    - - -

    -The following example calls the load function of the popular -js-yaml package on data that comes from an HTTP request and -hence is inherently unsafe. -

    - -

    -Using the safeLoad function instead (which does not deserialize -YAML-encoded functions) removes the vulnerability. -

    - -
    - - - -
  • -OWASP vulnerability description: -Deserialization of untrusted data. -
  • -
  • -OWASP guidance on deserializing objects: -Deserialization Cheat Sheet. -
  • -
  • -Neal Poole: -Code Execution via YAML in JS-YAML Node.js Module. -
  • -
    - -
    + + \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-611/Xxe.inc.qhelp b/javascript/ql/src/Security/CWE-611/Xxe.inc.qhelp new file mode 100644 index 000000000000..1e859eb121fa --- /dev/null +++ b/javascript/ql/src/Security/CWE-611/Xxe.inc.qhelp @@ -0,0 +1,57 @@ + + + + +

    +Parsing untrusted XML files with a weakly configured XML parser may lead to an +XML External Entity (XXE) attack. This type of attack uses external entity references +to access arbitrary files on a system, carry out denial-of-service (DoS) attacks, or server-side +request forgery. Even when the result of parsing is not returned to the user, DoS attacks are still possible +and out-of-band data retrieval techniques may allow attackers to steal sensitive data. +

    +
    + + +

    +The easiest way to prevent XXE attacks is to disable external entity handling when +parsing untrusted data. How this is done depends on the library being used. Note that some +libraries, such as recent versions of libxml, disable entity expansion by default, +so unless you have explicitly enabled entity expansion, no further action needs to be taken. +

    +
    + + +

    +The following example uses the libxml XML parser to parse a string xmlSrc. +If that string is from an untrusted source, this code may be vulnerable to an XXE attack, since +the parser is invoked with the noent option set to true: +

    + + +

    +To guard against XXE attacks, the noent option should be omitted or set to +false. This means that no entity expansion is undertaken at all, not even for standard +internal entities such as & or >. If desired, these +entities can be expanded in a separate step using utility functions provided by libraries such +as underscore, +lodash or +he. +

    + +
    + + +
  • +OWASP: +XML External Entity (XXE) Processing. +
  • +
  • +Timothy Morgen: +XML Schema, DTD, and Entity Attacks. +
  • +
  • +Timur Yunusov, Alexey Osipov: +XML Out-Of-Band Data Retrieval. +
  • +
    +
    diff --git a/javascript/ql/src/Security/CWE-611/Xxe.qhelp b/javascript/ql/src/Security/CWE-611/Xxe.qhelp index 1e859eb121fa..c0b8f28f0d4e 100644 --- a/javascript/ql/src/Security/CWE-611/Xxe.qhelp +++ b/javascript/ql/src/Security/CWE-611/Xxe.qhelp @@ -1,57 +1,6 @@ - + - - -

    -Parsing untrusted XML files with a weakly configured XML parser may lead to an -XML External Entity (XXE) attack. This type of attack uses external entity references -to access arbitrary files on a system, carry out denial-of-service (DoS) attacks, or server-side -request forgery. Even when the result of parsing is not returned to the user, DoS attacks are still possible -and out-of-band data retrieval techniques may allow attackers to steal sensitive data. -

    -
    - - -

    -The easiest way to prevent XXE attacks is to disable external entity handling when -parsing untrusted data. How this is done depends on the library being used. Note that some -libraries, such as recent versions of libxml, disable entity expansion by default, -so unless you have explicitly enabled entity expansion, no further action needs to be taken. -

    -
    - - -

    -The following example uses the libxml XML parser to parse a string xmlSrc. -If that string is from an untrusted source, this code may be vulnerable to an XXE attack, since -the parser is invoked with the noent option set to true: -

    - - -

    -To guard against XXE attacks, the noent option should be omitted or set to -false. This means that no entity expansion is undertaken at all, not even for standard -internal entities such as & or >. If desired, these -entities can be expanded in a separate step using utility functions provided by libraries such -as underscore, -lodash or -he. -

    - -
    - - -
  • -OWASP: -XML External Entity (XXE) Processing. -
  • -
  • -Timothy Morgen: -XML Schema, DTD, and Entity Attacks. -
  • -
  • -Timur Yunusov, Alexey Osipov: -XML Out-Of-Band Data Retrieval. -
  • -
    -
    + + \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-643/XpathInjection.inc.qhelp b/javascript/ql/src/Security/CWE-643/XpathInjection.inc.qhelp new file mode 100644 index 000000000000..3378b9b40785 --- /dev/null +++ b/javascript/ql/src/Security/CWE-643/XpathInjection.inc.qhelp @@ -0,0 +1,40 @@ + + + +

    +If an XPath expression is built using string concatenation, and the components of the concatenation +include user input, it makes it very easy for a user to create a malicious XPath expression. +

    +
    + + +

    +If user input must be included in an XPath expression, either sanitize the data or use variable +references to safely embed it without altering the structure of the expression. +

    +
    + + +

    +In this example, the code accepts a user name specified by the user, and uses this +unvalidated and unsanitized value in an XPath expression constructed using the xpath +package. This is vulnerable to the user providing special characters or string sequences +that change the meaning of the XPath expression to search for different values. +

    + + +

    +Instead, embed the user input using the variable replacement mechanism offered +by xpath: +

    + +
    + + +
  • OWASP: Testing for XPath Injection.
  • +
  • OWASP: XPath Injection.
  • +
  • npm: xpath.
  • +
    +
    diff --git a/javascript/ql/src/Security/CWE-643/XpathInjection.qhelp b/javascript/ql/src/Security/CWE-643/XpathInjection.qhelp index 3378b9b40785..8915475defe7 100644 --- a/javascript/ql/src/Security/CWE-643/XpathInjection.qhelp +++ b/javascript/ql/src/Security/CWE-643/XpathInjection.qhelp @@ -1,40 +1,6 @@ +"-//Semmle//qhelp//EN" +"qhelp.dtd"> - -

    -If an XPath expression is built using string concatenation, and the components of the concatenation -include user input, it makes it very easy for a user to create a malicious XPath expression. -

    -
    - - -

    -If user input must be included in an XPath expression, either sanitize the data or use variable -references to safely embed it without altering the structure of the expression. -

    -
    - - -

    -In this example, the code accepts a user name specified by the user, and uses this -unvalidated and unsanitized value in an XPath expression constructed using the xpath -package. This is vulnerable to the user providing special characters or string sequences -that change the meaning of the XPath expression to search for different values. -

    - - -

    -Instead, embed the user input using the variable replacement mechanism offered -by xpath: -

    - -
    - - -
  • OWASP: Testing for XPath Injection.
  • -
  • OWASP: XPath Injection.
  • -
  • npm: xpath.
  • -
    -
    + + \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-730/RegExpInjection.inc.qhelp b/javascript/ql/src/Security/CWE-730/RegExpInjection.inc.qhelp new file mode 100644 index 000000000000..6cd1120d07bb --- /dev/null +++ b/javascript/ql/src/Security/CWE-730/RegExpInjection.inc.qhelp @@ -0,0 +1,48 @@ + + + + +

    +Constructing a regular expression with unsanitized user input is dangerous as a malicious user may +be able to modify the meaning of the expression. In particular, such a user may be able to provide +a regular expression fragment that takes exponential time in the worst case, and use that to +perform a Denial of Service attack. +

    +
    + + +

    +Before embedding user input into a regular expression, use a sanitization function such as +lodash's _.escapeRegExp to escape meta-characters that have special meaning. +

    +
    + + +

    +The following example shows a HTTP request parameter that is used to construct a regular expression +without sanitizing it first: +

    + +

    +Instead, the request parameter should be sanitized first, for example using the function +_.escapeRegExp from the lodash package. This ensures that the user cannot insert +characters which have a special meaning in regular expressions. +

    + +
    + + +
  • +OWASP: +Regular expression Denial of Service - ReDoS. +
  • +
  • +Wikipedia: ReDoS. +
  • +
  • +npm: lodash. +
  • +
    +
    diff --git a/javascript/ql/src/Security/CWE-730/RegExpInjection.qhelp b/javascript/ql/src/Security/CWE-730/RegExpInjection.qhelp index 6cd1120d07bb..0340b9c047c3 100644 --- a/javascript/ql/src/Security/CWE-730/RegExpInjection.qhelp +++ b/javascript/ql/src/Security/CWE-730/RegExpInjection.qhelp @@ -1,48 +1,6 @@ +"-//Semmle//qhelp//EN" +"qhelp.dtd"> - - -

    -Constructing a regular expression with unsanitized user input is dangerous as a malicious user may -be able to modify the meaning of the expression. In particular, such a user may be able to provide -a regular expression fragment that takes exponential time in the worst case, and use that to -perform a Denial of Service attack. -

    -
    - - -

    -Before embedding user input into a regular expression, use a sanitization function such as -lodash's _.escapeRegExp to escape meta-characters that have special meaning. -

    -
    - - -

    -The following example shows a HTTP request parameter that is used to construct a regular expression -without sanitizing it first: -

    - -

    -Instead, the request parameter should be sanitized first, for example using the function -_.escapeRegExp from the lodash package. This ensures that the user cannot insert -characters which have a special meaning in regular expressions. -

    - -
    - - -
  • -OWASP: -Regular expression Denial of Service - ReDoS. -
  • -
  • -Wikipedia: ReDoS. -
  • -
  • -npm: lodash. -
  • -
    -
    + + \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-770/ResourceExhaustion.inc.qhelp b/javascript/ql/src/Security/CWE-770/ResourceExhaustion.inc.qhelp new file mode 100644 index 000000000000..4c07009f29ee --- /dev/null +++ b/javascript/ql/src/Security/CWE-770/ResourceExhaustion.inc.qhelp @@ -0,0 +1,115 @@ + + + + + +

    + + Applications are constrained by how many resources they can make use + of. Failing to respect these constraints may cause the application to + be unresponsive or crash. It is therefore problematic if attackers + can control the sizes or lifetimes of allocated objects. + +

    + +
    + + + +

    + + Ensure that attackers can not control object sizes and their + lifetimes. If object sizes and lifetimes must be controlled by + external parties, ensure you restrict the object sizes and lifetimes so that + they are within acceptable ranges. + +

    + +
    + + + +

    + + The following example allocates a buffer with a user-controlled + size. + +

    + + + +

    + + This is problematic since an attacker can choose a size + that makes the application run out of memory. Even worse, in older + versions of Node.js, this could leak confidential memory. + + To prevent such attacks, limit the buffer size: + +

    + + + +
    + + + +

    + + As another example, consider an application that allocates an + array with a user-controlled size, and then fills it with values: + +

    + + + +

    + The allocation of the array itself is not problematic since arrays are + allocated sparsely, but the subsequent filling of the array will take + a long time, causing the application to be unresponsive, or even run + out of memory. + + Again, a limit on the size will prevent the attack: + +

    + + + +
    + + + +

    + + Finally, the following example lets a user choose a delay after + which a function is executed: + +

    + + + +

    + + This is problematic because a large delay essentially makes the + application wait indefinitely before executing the function. Repeated + registrations of such delays will therefore use up all of the memory + in the application. + + A limit on the delay will prevent the attack: + +

    + + + + +
    + + +
  • + Wikipedia: Denial-of-service attack. +
  • +
    + +
    diff --git a/javascript/ql/src/Security/CWE-770/ResourceExhaustion.qhelp b/javascript/ql/src/Security/CWE-770/ResourceExhaustion.qhelp index 4c07009f29ee..88469add7b4e 100644 --- a/javascript/ql/src/Security/CWE-770/ResourceExhaustion.qhelp +++ b/javascript/ql/src/Security/CWE-770/ResourceExhaustion.qhelp @@ -2,114 +2,5 @@ "-//Semmle//qhelp//EN" "qhelp.dtd"> - - - -

    - - Applications are constrained by how many resources they can make use - of. Failing to respect these constraints may cause the application to - be unresponsive or crash. It is therefore problematic if attackers - can control the sizes or lifetimes of allocated objects. - -

    - -
    - - - -

    - - Ensure that attackers can not control object sizes and their - lifetimes. If object sizes and lifetimes must be controlled by - external parties, ensure you restrict the object sizes and lifetimes so that - they are within acceptable ranges. - -

    - -
    - - - -

    - - The following example allocates a buffer with a user-controlled - size. - -

    - - - -

    - - This is problematic since an attacker can choose a size - that makes the application run out of memory. Even worse, in older - versions of Node.js, this could leak confidential memory. - - To prevent such attacks, limit the buffer size: - -

    - - - -
    - - - -

    - - As another example, consider an application that allocates an - array with a user-controlled size, and then fills it with values: - -

    - - - -

    - The allocation of the array itself is not problematic since arrays are - allocated sparsely, but the subsequent filling of the array will take - a long time, causing the application to be unresponsive, or even run - out of memory. - - Again, a limit on the size will prevent the attack: - -

    - - - -
    - - - -

    - - Finally, the following example lets a user choose a delay after - which a function is executed: - -

    - - - -

    - - This is problematic because a large delay essentially makes the - application wait indefinitely before executing the function. Repeated - registrations of such delays will therefore use up all of the memory - in the application. - - A limit on the delay will prevent the attack: - -

    - - - - -
    - - -
  • - Wikipedia: Denial-of-service attack. -
  • -
    - -
    + + \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-776/XmlBomb.inc.qhelp b/javascript/ql/src/Security/CWE-776/XmlBomb.inc.qhelp new file mode 100644 index 000000000000..c0714b3f96f9 --- /dev/null +++ b/javascript/ql/src/Security/CWE-776/XmlBomb.inc.qhelp @@ -0,0 +1,60 @@ + + + + +

    +Parsing untrusted XML files with a weakly configured XML parser may be vulnerable to +denial-of-service (DoS) attacks exploiting uncontrolled internal entity expansion. +

    +

    +In XML, so-called internal entities are a mechanism for introducing an abbreviation +for a piece of text or part of a document. When a parser that has been configured +to expand entities encounters a reference to an internal entity, it replaces the entity +by the data it represents. The replacement text may itself contain other entity references, +which are expanded recursively. This means that entity expansion can increase document size +dramatically. +

    +

    +If untrusted XML is parsed with entity expansion enabled, a malicious attacker could +submit a document that contains very deeply nested entity definitions, causing the parser +to take a very long time or use large amounts of memory. This is sometimes called an +XML bomb attack. +

    +
    + + +

    +The safest way to prevent XML bomb attacks is to disable entity expansion when parsing untrusted +data. How this is done depends on the library being used. Note that some libraries, such as +recent versions of libxmljs (though not its SAX parser API), disable entity expansion +by default, so unless you have explicitly enabled entity expansion, no further action is needed. +

    +
    + + +

    +The following example uses the XML parser provided by the node-expat package to +parse a string xmlSrc. If that string is from an untrusted source, this code may be +vulnerable to a DoS attack, since node-expat expands internal entities by default: +

    + + +

    +At the time of writing, node-expat does not provide a way of controlling entity +expansion, but the example could be rewritten to use the sax package instead, +which only expands standard entities such as &: +

    + +
    + + +
  • +Wikipedia: +Billion Laughs. +
  • +
  • +Bryan Sullivan: +Security Briefs - XML Denial of Service Attacks and Defenses. +
  • +
    +
    diff --git a/javascript/ql/src/Security/CWE-776/XmlBomb.qhelp b/javascript/ql/src/Security/CWE-776/XmlBomb.qhelp index c0714b3f96f9..ae26e96440fa 100644 --- a/javascript/ql/src/Security/CWE-776/XmlBomb.qhelp +++ b/javascript/ql/src/Security/CWE-776/XmlBomb.qhelp @@ -1,60 +1,6 @@ - + - - -

    -Parsing untrusted XML files with a weakly configured XML parser may be vulnerable to -denial-of-service (DoS) attacks exploiting uncontrolled internal entity expansion. -

    -

    -In XML, so-called internal entities are a mechanism for introducing an abbreviation -for a piece of text or part of a document. When a parser that has been configured -to expand entities encounters a reference to an internal entity, it replaces the entity -by the data it represents. The replacement text may itself contain other entity references, -which are expanded recursively. This means that entity expansion can increase document size -dramatically. -

    -

    -If untrusted XML is parsed with entity expansion enabled, a malicious attacker could -submit a document that contains very deeply nested entity definitions, causing the parser -to take a very long time or use large amounts of memory. This is sometimes called an -XML bomb attack. -

    -
    - - -

    -The safest way to prevent XML bomb attacks is to disable entity expansion when parsing untrusted -data. How this is done depends on the library being used. Note that some libraries, such as -recent versions of libxmljs (though not its SAX parser API), disable entity expansion -by default, so unless you have explicitly enabled entity expansion, no further action is needed. -

    -
    - - -

    -The following example uses the XML parser provided by the node-expat package to -parse a string xmlSrc. If that string is from an untrusted source, this code may be -vulnerable to a DoS attack, since node-expat expands internal entities by default: -

    - - -

    -At the time of writing, node-expat does not provide a way of controlling entity -expansion, but the example could be rewritten to use the sax package instead, -which only expands standard entities such as &: -

    - -
    - - -
  • -Wikipedia: -Billion Laughs. -
  • -
  • -Bryan Sullivan: -Security Briefs - XML Denial of Service Attacks and Defenses. -
  • -
    -
    + + \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-807/ConditionalBypass.inc.qhelp b/javascript/ql/src/Security/CWE-807/ConditionalBypass.inc.qhelp new file mode 100644 index 000000000000..3add2649c39e --- /dev/null +++ b/javascript/ql/src/Security/CWE-807/ConditionalBypass.inc.qhelp @@ -0,0 +1,27 @@ + + + +

    + + Using user-controlled data in a permissions check may + allow a user to gain unauthorized access to protected functionality or + data. + +

    + +
    + + + + + + + + + + + + +
    diff --git a/javascript/ql/src/Security/CWE-807/ConditionalBypass.qhelp b/javascript/ql/src/Security/CWE-807/ConditionalBypass.qhelp index 3add2649c39e..01a91a8d1619 100644 --- a/javascript/ql/src/Security/CWE-807/ConditionalBypass.qhelp +++ b/javascript/ql/src/Security/CWE-807/ConditionalBypass.qhelp @@ -2,26 +2,5 @@ "-//Semmle//qhelp//EN" "qhelp.dtd"> - -

    - - Using user-controlled data in a permissions check may - allow a user to gain unauthorized access to protected functionality or - data. - -

    - -
    - - - - - - - - - - - - -
    + + \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-807/ConditionalBypass.ql b/javascript/ql/src/Security/CWE-807/ConditionalBypass.ql index 2fa13629368e..492dc5b8b6e7 100644 --- a/javascript/ql/src/Security/CWE-807/ConditionalBypass.ql +++ b/javascript/ql/src/Security/CWE-807/ConditionalBypass.ql @@ -15,99 +15,6 @@ import javascript import semmle.javascript.security.dataflow.ConditionalBypassQuery import DataFlow::PathGraph -/** - * Holds if the value of `nd` flows into `guard`. - */ -predicate flowsToGuardExpr(DataFlow::Node nd, SensitiveActionGuardConditional guard) { - nd = guard or - flowsToGuardExpr(nd.getASuccessor(), guard) -} - -/** - * A comparison that guards a sensitive action, e.g. the comparison in: - * `var ok = x == y; if (ok) login()`. - */ -class SensitiveActionGuardComparison extends Comparison { - SensitiveActionGuardConditional guard; - - SensitiveActionGuardComparison() { flowsToGuardExpr(DataFlow::valueNode(this), guard) } - - /** - * Gets the guard that uses this comparison. - */ - SensitiveActionGuardConditional getGuard() { result = guard } -} - -/** - * An intermediary sink to enable reuse of the taint configuration. - * This sink should not be presented to the client of this query. - */ -class SensitiveActionGuardComparisonOperand extends Sink { - SensitiveActionGuardComparison comparison; - - SensitiveActionGuardComparisonOperand() { asExpr() = comparison.getAnOperand() } - - override SensitiveAction getAction() { result = comparison.getGuard().getAction() } -} - -/** - * Holds if `sink` guards `action`, and `source` taints `sink`. - * - * If flow from `source` taints `sink`, then an attacker can - * control if `action` should be executed or not. - */ -predicate isTaintedGuardForSensitiveAction( - DataFlow::PathNode sink, DataFlow::PathNode source, SensitiveAction action -) { - action = sink.getNode().(Sink).getAction() and - // exclude the intermediary sink - not sink.getNode() instanceof SensitiveActionGuardComparisonOperand and - exists(Configuration cfg | - // ordinary taint tracking to a guard - cfg.hasFlowPath(source, sink) - or - // taint tracking to both operands of a guard comparison - exists( - SensitiveActionGuardComparison cmp, DataFlow::PathNode lSource, DataFlow::PathNode rSource, - DataFlow::PathNode lSink, DataFlow::PathNode rSink - | - sink.getNode() = cmp.getGuard() and - cfg.hasFlowPath(lSource, lSink) and - lSink.getNode() = DataFlow::valueNode(cmp.getLeftOperand()) and - cfg.hasFlowPath(rSource, rSink) and - rSink.getNode() = DataFlow::valueNode(cmp.getRightOperand()) - | - source = lSource or - source = rSource - ) - ) -} - -/** - * Holds if `e` effectively guards access to `action` by returning or throwing early. - * - * Example: `if (e) return; action(x)`. - */ -predicate isEarlyAbortGuard(DataFlow::PathNode e, SensitiveAction action) { - exists(IfStmt guard | - // `e` is in the condition of an if-statement ... - e.getNode().(Sink).asExpr().getParentExpr*() = guard.getCondition() and - // ... where the then-branch always throws or returns - exists(Stmt abort | - abort instanceof ThrowStmt or - abort instanceof ReturnStmt - | - abort.nestedIn(guard) and - abort.getBasicBlock().(ReachableBasicBlock).postDominates(guard.getThen().getBasicBlock()) - ) and - // ... and the else-branch does not exist - not exists(guard.getElse()) - | - // ... and `action` is outside the if-statement - not action.asExpr().getEnclosingStmt().nestedIn(guard) - ) -} - from DataFlow::PathNode source, DataFlow::PathNode sink, SensitiveAction action where isTaintedGuardForSensitiveAction(sink, source, action) and diff --git a/javascript/ql/src/Security/CWE-915/PrototypePollutingAssignment.inc.qhelp b/javascript/ql/src/Security/CWE-915/PrototypePollutingAssignment.inc.qhelp new file mode 100644 index 000000000000..b88457431bfa --- /dev/null +++ b/javascript/ql/src/Security/CWE-915/PrototypePollutingAssignment.inc.qhelp @@ -0,0 +1,62 @@ + + + + +

    + Most JavaScript objects inherit the properties of the built-in Object.prototype object. + Prototype pollution is a type of vulnerability in which an attacker is able to modify Object.prototype. + Since most objects inherit from the compromised Object.prototype object, the attacker can use this + to tamper with the application logic, and often escalate to remote code execution or cross-site scripting. +

    + +

    + One way to cause prototype pollution is by modifying an object obtained via a user-controlled property name. + Most objects have a special __proto__ property that refers to Object.prototype. + An attacker can abuse this special property to trick the application into performing unintended modifications + of Object.prototype. +

    +
    + + +

    + Use an associative data structure that is resilient to untrusted key values, such as a Map. + In some cases, a prototype-less object created with Object.create(null) + may be preferable. +

    +

    + Alternatively, restrict the computed property name so it can't clash with a built-in property, either by + prefixing it with a constant string, or by rejecting inputs that don't conform to the expected format. +

    +
    + + +

    + In the example below, the untrusted value req.params.id is used as the property name + req.session.todos[id]. If a malicious user passes in the ID value __proto__, + the variable todo will then refer to Object.prototype. + Finally, the modification of todo then allows the attacker to inject arbitrary properties + onto Object.prototype. +

    + + + +

    + One way to fix this is to use Map objects to associate key/value pairs + instead of regular objects, as shown below: +

    + + + +
    + + +
  • MDN: + Object.prototype.__proto__ +
  • +
  • MDN: + Map +
  • +
    +
    diff --git a/javascript/ql/src/Security/CWE-915/PrototypePollutingAssignment.qhelp b/javascript/ql/src/Security/CWE-915/PrototypePollutingAssignment.qhelp index b88457431bfa..880c006027cf 100644 --- a/javascript/ql/src/Security/CWE-915/PrototypePollutingAssignment.qhelp +++ b/javascript/ql/src/Security/CWE-915/PrototypePollutingAssignment.qhelp @@ -2,61 +2,5 @@ "-//Semmle//qhelp//EN" "qhelp.dtd"> - - -

    - Most JavaScript objects inherit the properties of the built-in Object.prototype object. - Prototype pollution is a type of vulnerability in which an attacker is able to modify Object.prototype. - Since most objects inherit from the compromised Object.prototype object, the attacker can use this - to tamper with the application logic, and often escalate to remote code execution or cross-site scripting. -

    - -

    - One way to cause prototype pollution is by modifying an object obtained via a user-controlled property name. - Most objects have a special __proto__ property that refers to Object.prototype. - An attacker can abuse this special property to trick the application into performing unintended modifications - of Object.prototype. -

    -
    - - -

    - Use an associative data structure that is resilient to untrusted key values, such as a Map. - In some cases, a prototype-less object created with Object.create(null) - may be preferable. -

    -

    - Alternatively, restrict the computed property name so it can't clash with a built-in property, either by - prefixing it with a constant string, or by rejecting inputs that don't conform to the expected format. -

    -
    - - -

    - In the example below, the untrusted value req.params.id is used as the property name - req.session.todos[id]. If a malicious user passes in the ID value __proto__, - the variable todo will then refer to Object.prototype. - Finally, the modification of todo then allows the attacker to inject arbitrary properties - onto Object.prototype. -

    - - - -

    - One way to fix this is to use Map objects to associate key/value pairs - instead of regular objects, as shown below: -

    - - - -
    - - -
  • MDN: - Object.prototype.__proto__ -
  • -
  • MDN: - Map -
  • -
    -
    + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.qhelp new file mode 100644 index 000000000000..db81a66ef3fe --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.ql new file mode 100644 index 000000000000..dff265363191 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.ql @@ -0,0 +1,22 @@ +/** + * @name Untrusted data passed to external API with additional heuristic sources + * @description Data provided remotely is used in this external API without sanitization, which could be a security risk. + * @id js/untrusted-data-to-external-api-more-sources + * @kind path-problem + * @precision low + * @problem.severity error + * @security-severity 7.8 + * @tags experimental + * security external/cwe/cwe-20 + */ + +import javascript +import semmle.javascript.security.dataflow.ExternalAPIUsedWithUntrustedDataQuery +import DataFlow::PathGraph +import semmle.javascript.heuristics.AdditionalSources + +from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink +where config.hasFlowPath(source, sink) and source.getNode() instanceof HeuristicSource +select sink, source, sink, + "Call to " + sink.getNode().(Sink).getApiName() + " with untrusted data from $@.", source, + source.toString() diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-078/CommandInjection.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-078/CommandInjection.qhelp new file mode 100644 index 000000000000..c8a9b1be61ef --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-078/CommandInjection.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-078/CommandInjection.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-078/CommandInjection.ql new file mode 100644 index 000000000000..b21c86fc50a6 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-078/CommandInjection.ql @@ -0,0 +1,35 @@ +/** + * @name Uncontrolled command line with additional heuristic sources + * @description Using externally controlled strings in a command line may allow a malicious + * user to change the meaning of the command. + * @kind path-problem + * @problem.severity error + * @security-severity 9.8 + * @precision high + * @id js/command-line-injection-more-sources + * @tags experimental + * correctness + * security + * external/cwe/cwe-078 + * external/cwe/cwe-088 + */ + +import javascript +import semmle.javascript.security.dataflow.CommandInjectionQuery +import DataFlow::PathGraph +import semmle.javascript.heuristics.AdditionalSources + +from + Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, DataFlow::Node highlight, + Source sourceNode +where + cfg.hasFlowPath(source, sink) and + ( + if cfg.isSinkWithHighlight(sink.getNode(), _) + then cfg.isSinkWithHighlight(sink.getNode(), highlight) + else highlight = sink.getNode() + ) and + sourceNode = source.getNode() and + source.getNode() instanceof HeuristicSource +select highlight, source, sink, "This command line depends on a $@.", source.getNode(), + "user-provided value" diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-079/Xss.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-079/Xss.qhelp new file mode 100644 index 000000000000..6519f06c7e72 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-079/Xss.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-079/Xss.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-079/Xss.ql new file mode 100644 index 000000000000..e93cd7e6ca5c --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-079/Xss.ql @@ -0,0 +1,25 @@ +/** + * @name Client-side cross-site scripting with additional heuristic sources + * @description Writing user input directly to the DOM allows for + * a cross-site scripting vulnerability. + * @kind path-problem + * @problem.severity error + * @security-severity 6.1 + * @precision high + * @id js/xss-more-sources + * @tags experimental + * security + * external/cwe/cwe-079 + * external/cwe/cwe-116 + */ + +import javascript +import semmle.javascript.security.dataflow.DomBasedXssQuery +import DataFlow::PathGraph +import semmle.javascript.heuristics.AdditionalSources + +from DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) and source.getNode() instanceof HeuristicSource +select sink.getNode(), source, sink, + sink.getNode().(Sink).getVulnerabilityKind() + " vulnerability due to $@.", source.getNode(), + "user-provided value" diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-089/SqlInjection.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-089/SqlInjection.qhelp new file mode 100644 index 000000000000..9e5e0d6074f2 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-089/SqlInjection.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-089/SqlInjection.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-089/SqlInjection.ql new file mode 100644 index 000000000000..a2c4a4158bc7 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-089/SqlInjection.ql @@ -0,0 +1,32 @@ +/** + * @name Database query built from user-controlled sources with additional heuristic sources + * @description Building a database query from user-controlled sources is vulnerable to insertion of + * malicious code by the user. + * @kind path-problem + * @problem.severity error + * @security-severity 8.8 + * @precision high + * @id js/sql-injection-more-sources + * @tags experimental + * security + * external/cwe/cwe-089 + * external/cwe/cwe-090 + * external/cwe/cwe-943 + */ + +import javascript +import semmle.javascript.security.dataflow.SqlInjectionQuery as SqlInjection +import semmle.javascript.security.dataflow.NosqlInjectionQuery as NosqlInjection +import DataFlow::PathGraph +import semmle.javascript.heuristics.AdditionalSources + +from DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where + ( + cfg instanceof SqlInjection::Configuration or + cfg instanceof NosqlInjection::Configuration + ) and + cfg.hasFlowPath(source, sink) and + source.getNode() instanceof HeuristicSource +select sink.getNode(), source, sink, "This query depends on a $@.", source.getNode(), + "user-provided value" diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-094/CodeInjection.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-094/CodeInjection.qhelp new file mode 100644 index 000000000000..41ede7eab669 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-094/CodeInjection.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-094/CodeInjection.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-094/CodeInjection.ql new file mode 100644 index 000000000000..89d7d253f413 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-094/CodeInjection.ql @@ -0,0 +1,26 @@ +/** + * @name Code injection with additional heuristic sources + * @description Interpreting unsanitized user input as code allows a malicious user arbitrary + * code execution. + * @kind path-problem + * @problem.severity error + * @security-severity 9.3 + * @precision high + * @id js/code-injection-more-sources + * @tags experimental + * security + * external/cwe/cwe-094 + * external/cwe/cwe-095 + * external/cwe/cwe-079 + * external/cwe/cwe-116 + */ + +import javascript +import semmle.javascript.security.dataflow.CodeInjectionQuery +import DataFlow::PathGraph +import semmle.javascript.heuristics.AdditionalSources + +from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) and source.getNode() instanceof HeuristicSource +select sink.getNode(), source, sink, sink.getNode().(Sink).getMessagePrefix() + " depends on a $@.", + source.getNode(), "user-provided value" diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-117/LogInjection.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-117/LogInjection.qhelp new file mode 100644 index 000000000000..96f209fd3352 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-117/LogInjection.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-117/LogInjection.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-117/LogInjection.ql new file mode 100644 index 000000000000..534de9167725 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-117/LogInjection.ql @@ -0,0 +1,23 @@ +/** + * @name Log injection with additional heuristic sources + * @description Building log entries from user-controlled sources is vulnerable to + * insertion of forged log entries by a malicious user. + * @kind path-problem + * @problem.severity error + * @security-severity 7.8 + * @precision medium + * @id js/log-injection-more-sources + * @tags experimental + * security + * external/cwe/cwe-117 + */ + +import javascript +import DataFlow::PathGraph +import semmle.javascript.security.dataflow.LogInjectionQuery +import semmle.javascript.heuristics.AdditionalSources + +from LogInjectionConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink +where config.hasFlowPath(source, sink) and source.getNode() instanceof HeuristicSource +select sink.getNode(), source, sink, "Log entry depends on a $@.", source.getNode(), + "user-provided value" diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-134/TaintedFormatString.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-134/TaintedFormatString.qhelp new file mode 100644 index 000000000000..ba6985c58abc --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-134/TaintedFormatString.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-134/TaintedFormatString.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-134/TaintedFormatString.ql new file mode 100644 index 000000000000..883f8292c758 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-134/TaintedFormatString.ql @@ -0,0 +1,22 @@ +/** + * @name Use of externally-controlled format string with additional heuristic sources + * @description Using external input in format strings can lead to garbled output. + * @kind path-problem + * @problem.severity warning + * @security-severity 7.3 + * @precision high + * @id js/tainted-format-string-more-sources + * @tags experimental + * security + * external/cwe/cwe-134 + */ + +import javascript +import semmle.javascript.security.dataflow.TaintedFormatStringQuery +import DataFlow::PathGraph +import semmle.javascript.heuristics.AdditionalSources + +from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) and source.getNode() instanceof HeuristicSource +select sink.getNode(), source, sink, "Format string depends on a $@.", source.getNode(), + "user-provided value" diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-346/CorsMisconfigurationForCredentials.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-346/CorsMisconfigurationForCredentials.qhelp new file mode 100644 index 000000000000..1efd62362592 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-346/CorsMisconfigurationForCredentials.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-346/CorsMisconfigurationForCredentials.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-346/CorsMisconfigurationForCredentials.ql new file mode 100644 index 000000000000..3448e4e99b62 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-346/CorsMisconfigurationForCredentials.ql @@ -0,0 +1,25 @@ +/** + * @name CORS misconfiguration for credentials transfer with additional heuristic sources + * @description Misconfiguration of CORS HTTP headers allows for leaks of secret credentials. + * @kind path-problem + * @problem.severity error + * @security-severity 7.5 + * @precision high + * @id js/cors-misconfiguration-for-credentials-more-sources + * @tags experimental + * security + * external/cwe/cwe-346 + * external/cwe/cwe-639 + * external/cwe/cwe-942 + */ + +import javascript +import semmle.javascript.security.dataflow.CorsMisconfigurationForCredentialsQuery +import DataFlow::PathGraph +import semmle.javascript.heuristics.AdditionalSources + +from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) and source.getNode() instanceof HeuristicSource +select sink.getNode(), source, sink, "$@ leak vulnerability due to a $@.", + sink.getNode().(Sink).getCredentialsHeader(), "Credential", source.getNode(), + "misconfigured CORS header value" diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-400/RemotePropertyInjection.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-400/RemotePropertyInjection.qhelp new file mode 100644 index 000000000000..854b92ab9754 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-400/RemotePropertyInjection.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-400/RemotePropertyInjection.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-400/RemotePropertyInjection.ql new file mode 100644 index 000000000000..fd707ae8faa4 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-400/RemotePropertyInjection.ql @@ -0,0 +1,24 @@ +/** + * @name Remote property injection with additional heuristic sources + * @description Allowing writes to arbitrary properties of an object may lead to + * denial-of-service attacks. + * @kind path-problem + * @problem.severity warning + * @security-severity 7.5 + * @precision medium + * @id js/remote-property-injection-more-sources + * @tags experimental + * security + * external/cwe/cwe-250 + * external/cwe/cwe-400 + */ + +import javascript +import semmle.javascript.security.dataflow.RemotePropertyInjectionQuery +import DataFlow::PathGraph +import semmle.javascript.heuristics.AdditionalSources + +from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) and source.getNode() instanceof HeuristicSource +select sink.getNode(), source, sink, sink.getNode().(Sink).getMessage() + " depends on a $@.", + source.getNode(), "user-provided value" diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-502/UnsafeDeserialization.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-502/UnsafeDeserialization.qhelp new file mode 100644 index 000000000000..ec372c77e7c3 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-502/UnsafeDeserialization.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-502/UnsafeDeserialization.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-502/UnsafeDeserialization.ql new file mode 100644 index 000000000000..24939f49b0dc --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-502/UnsafeDeserialization.ql @@ -0,0 +1,23 @@ +/** + * @name Deserialization of user-controlled data with additional heuristic sources + * @description Deserializing user-controlled data may allow attackers to + * execute arbitrary code. + * @kind path-problem + * @problem.severity warning + * @security-severity 9.8 + * @precision high + * @id js/unsafe-deserialization-more-sources + * @tags experimental + * security + * external/cwe/cwe-502 + */ + +import javascript +import semmle.javascript.security.dataflow.UnsafeDeserializationQuery +import DataFlow::PathGraph +import semmle.javascript.heuristics.AdditionalSources + +from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) and source.getNode() instanceof HeuristicSource +select sink.getNode(), source, sink, "Unsafe deserialization depends on a $@.", source.getNode(), + "user-provided value" diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-611/Xxe.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-611/Xxe.qhelp new file mode 100644 index 000000000000..87c53478ea54 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-611/Xxe.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-611/Xxe.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-611/Xxe.ql new file mode 100644 index 000000000000..cbfaa33ca518 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-611/Xxe.ql @@ -0,0 +1,25 @@ +/** + * @name XML external entity expansion with additional heuristic sources + * @description Parsing user input as an XML document with external + * entity expansion is vulnerable to XXE attacks. + * @kind path-problem + * @problem.severity error + * @security-severity 9.1 + * @precision high + * @id js/xxe-more-sources + * @tags experimental + * security + * external/cwe/cwe-611 + * external/cwe/cwe-827 + */ + +import javascript +import semmle.javascript.security.dataflow.XxeQuery +import DataFlow::PathGraph +import semmle.javascript.heuristics.AdditionalSources + +from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) and source.getNode() instanceof HeuristicSource +select sink.getNode(), source, sink, + "XML parsing depends on a $@ without guarding against external entity expansion.", + source.getNode(), "user-provided value" diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-643/XpathInjection.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-643/XpathInjection.qhelp new file mode 100644 index 000000000000..fc4718c08fd1 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-643/XpathInjection.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-643/XpathInjection.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-643/XpathInjection.ql new file mode 100644 index 000000000000..0a00511c86b6 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-643/XpathInjection.ql @@ -0,0 +1,23 @@ +/** + * @name XPath injection with additional heuristic sources + * @description Building an XPath expression from user-controlled sources is vulnerable to insertion of + * malicious code by the user. + * @kind path-problem + * @problem.severity error + * @security-severity 9.8 + * @precision high + * @id js/xpath-injection-more-sources + * @tags experimental + * security + * external/cwe/cwe-643 + */ + +import javascript +import semmle.javascript.security.dataflow.XpathInjectionQuery +import DataFlow::PathGraph +import semmle.javascript.heuristics.AdditionalSources + +from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) and source.getNode() instanceof HeuristicSource +select sink.getNode(), source, sink, "XPath expression depends on a $@.", source.getNode(), + "user-provided value" diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-730/RegExpInjection.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-730/RegExpInjection.qhelp new file mode 100644 index 000000000000..09e1a7b08514 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-730/RegExpInjection.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-730/RegExpInjection.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-730/RegExpInjection.ql new file mode 100644 index 000000000000..de302e53871e --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-730/RegExpInjection.ql @@ -0,0 +1,25 @@ +/** + * @name Regular expression injection with additional heuristic sources + * @description User input should not be used in regular expressions without first being escaped, + * otherwise a malicious user may be able to inject an expression that could require + * exponential time on certain inputs. + * @kind path-problem + * @problem.severity error + * @security-severity 7.5 + * @precision high + * @id js/regex-injection-more-sources + * @tags experimental + * security + * external/cwe/cwe-730 + * external/cwe/cwe-400 + */ + +import javascript +import semmle.javascript.security.dataflow.RegExpInjectionQuery +import DataFlow::PathGraph +import semmle.javascript.heuristics.AdditionalSources + +from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) and source.getNode() instanceof HeuristicSource +select sink.getNode(), source, sink, "This regular expression is constructed from a $@.", + source.getNode(), "user-provided value" diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-770/ResourceExhaustion.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-770/ResourceExhaustion.qhelp new file mode 100644 index 000000000000..ee923465b889 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-770/ResourceExhaustion.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-770/ResourceExhaustion.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-770/ResourceExhaustion.ql new file mode 100644 index 000000000000..37e702b55e01 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-770/ResourceExhaustion.ql @@ -0,0 +1,24 @@ +/** + * @name Resource exhaustion with additional heuristic sources + * @description Allocating objects or timers with user-controlled + * sizes or durations can cause resource exhaustion. + * @kind path-problem + * @problem.severity warning + * @security-severity 7.5 + * @id js/resource-exhaustion-more-sources + * @precision high + * @tags experimental + * security + * external/cwe/cwe-400 + * external/cwe/cwe-770 + */ + +import javascript +import DataFlow::PathGraph +import semmle.javascript.security.dataflow.ResourceExhaustionQuery +import semmle.javascript.heuristics.AdditionalSources + +from Configuration dataflow, DataFlow::PathNode source, DataFlow::PathNode sink +where dataflow.hasFlowPath(source, sink) and source.getNode() instanceof HeuristicSource +select sink, source, sink, sink.getNode().(Sink).getProblemDescription() + " from a $@.", source, + "user-provided value" diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-776/XmlBomb.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-776/XmlBomb.qhelp new file mode 100644 index 000000000000..7a569ed6b871 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-776/XmlBomb.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-776/XmlBomb.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-776/XmlBomb.ql new file mode 100644 index 000000000000..1c05ba2424f0 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-776/XmlBomb.ql @@ -0,0 +1,25 @@ +/** + * @name XML internal entity expansion with additional heuristic sources + * @description Parsing user input as an XML document with arbitrary internal + * entity expansion is vulnerable to denial-of-service attacks. + * @kind path-problem + * @problem.severity warning + * @security-severity 7.5 + * @precision high + * @id js/xml-bomb-more-sources + * @tags experimental + * security + * external/cwe/cwe-776 + * external/cwe/cwe-400 + */ + +import javascript +import semmle.javascript.security.dataflow.XmlBombQuery +import DataFlow::PathGraph +import semmle.javascript.heuristics.AdditionalSources + +from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) and source.getNode() instanceof HeuristicSource +select sink.getNode(), source, sink, + "XML parsing depends on a $@ without guarding against uncontrolled entity expansion.", + source.getNode(), "user-provided value" diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-807/ConditionalBypass.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-807/ConditionalBypass.qhelp new file mode 100644 index 000000000000..8fdb0408acfc --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-807/ConditionalBypass.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-807/ConditionalBypass.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-807/ConditionalBypass.ql new file mode 100644 index 000000000000..6fe3ff742f3f --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-807/ConditionalBypass.ql @@ -0,0 +1,26 @@ +/** + * @name User-controlled bypass of security check with additional heuristic sources + * @description Conditions that the user controls are not suited for making security-related decisions. + * @kind path-problem + * @problem.severity error + * @security-severity 7.8 + * @precision medium + * @id js/user-controlled-bypass-more-sources + * @tags experimental + * security + * external/cwe/cwe-807 + * external/cwe/cwe-290 + */ + +import javascript +import semmle.javascript.security.dataflow.ConditionalBypassQuery +import DataFlow::PathGraph +import semmle.javascript.heuristics.AdditionalSources + +from DataFlow::PathNode source, DataFlow::PathNode sink, SensitiveAction action +where + isTaintedGuardForSensitiveAction(sink, source, action) and + not isEarlyAbortGuard(sink, action) and + source.getNode() instanceof HeuristicSource +select sink.getNode(), source, sink, "This condition guards a sensitive $@, but a $@ controls it.", + action, "action", source.getNode(), "user-provided value" diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-915/PrototypePollutingAssignment.qhelp b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-915/PrototypePollutingAssignment.qhelp new file mode 100644 index 000000000000..98d0779e1fc4 --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-915/PrototypePollutingAssignment.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-915/PrototypePollutingAssignment.ql b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-915/PrototypePollutingAssignment.ql new file mode 100644 index 000000000000..eae399ea00fe --- /dev/null +++ b/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-915/PrototypePollutingAssignment.ql @@ -0,0 +1,30 @@ +/** + * @name Prototype-polluting assignment with additional heuristic sources + * @description Modifying an object obtained via a user-controlled property name may + * lead to accidental mutation of the built-in Object prototype, + * and possibly escalate to remote code execution or cross-site scripting. + * @kind path-problem + * @problem.severity warning + * @security-severity 6.1 + * @precision high + * @id js/prototype-polluting-assignment-more-sources + * @tags experimental + * security + * external/cwe/cwe-078 + * external/cwe/cwe-079 + * external/cwe/cwe-094 + * external/cwe/cwe-400 + * external/cwe/cwe-471 + * external/cwe/cwe-915 + */ + +import javascript +import semmle.javascript.security.dataflow.PrototypePollutingAssignmentQuery +import DataFlow::PathGraph +import semmle.javascript.heuristics.AdditionalSources + +from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) and source.getNode() instanceof HeuristicSource +select sink, source, sink, + "This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@.", + source.getNode(), source.getNode().(Source).describe() diff --git a/ql/ql/src/codeql_ql/ast/Ast.qll b/ql/ql/src/codeql_ql/ast/Ast.qll index 7962421eb7f2..63b8833d55b6 100644 --- a/ql/ql/src/codeql_ql/ast/Ast.qll +++ b/ql/ql/src/codeql_ql/ast/Ast.qll @@ -206,6 +206,11 @@ class QueryDoc extends QLDoc { result = this.getContents().regexpCapture("(?s).*@kind ([\\w-]+)\\s.*", 1) } + /** Gets the @name for the query */ + string getQueryName() { + result = this.getContents().regexpCapture("(?s).*@name ([\\w-\\s]+)(?=\\n).*", 1) + } + /** Gets the id part (without language) of the @id */ string getQueryId() { result = this.getContents().regexpCapture("(?s).*@id (\\w+)/([\\w\\-]+)\\s.*", 2)