Merge pull request #6139 from erik-krogh/colors

Approved by esbena
2026-05-01 11:45:14 +02:00 · 2021-06-23 14:02:17 -07:00
parent ffdc752720 dbc8b9cf6a
commit c02c96369d
4 changed files with 271 additions and 0 deletions
--- a/javascript/ql/src/semmle/javascript/frameworks/Logging.qll
+++ b/javascript/ql/src/semmle/javascript/frameworks/Logging.qll
@@ -201,3 +201,139 @@ private class DebugLoggerCall extends LoggerCall, API::CallNode {

  override DataFlow::Node getAMessageComponent() { result = getAnArgument() }
 }
+
+/**
+ * A step through the [`ansi-colors`](https://https://npmjs.org/package/ansi-colors) library.
+ */
+class AnsiColorsStep extends TaintTracking::SharedTaintStep {
+  override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(API::CallNode call | call = API::moduleImport("ansi-colors").getAMember*().getACall() |
+      pred = call.getArgument(0) and
+      succ = call
+    )
+  }
+}
+
+/**
+ * A step through the [`colors`](https://npmjs.org/package/colors) library.
+ * This step ignores the `String.prototype` modifying part of the `colors` library.
+ */
+class ColorsStep extends TaintTracking::SharedTaintStep {
+  override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(API::CallNode call |
+      call =
+        API::moduleImport([
+            "colors",
+            // the `colors/safe` variant avoids modifying the prototype methods
+            "colors/safe"
+          ]).getAMember*().getACall()
+    |
+      pred = call.getArgument(0) and
+      succ = call
+    )
+  }
+}
+
+/**
+ * A step through the [`wrap-ansi`](https://npmjs.org/package/wrap-ansi) library.
+ */
+class WrapAnsiStep extends TaintTracking::SharedTaintStep {
+  override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(API::CallNode call | call = API::moduleImport("wrap-ansi").getACall() |
+      pred = call.getArgument(0) and
+      succ = call
+    )
+  }
+}
+
+/**
+ * A step through the [`colorette`](https://npmjs.org/package/colorette) library.
+ */
+class ColoretteStep extends TaintTracking::SharedTaintStep {
+  override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(API::CallNode call | call = API::moduleImport("colorette").getAMember().getACall() |
+      pred = call.getArgument(0) and
+      succ = call
+    )
+  }
+}
+
+/**
+ * A step through the [`cli-highlight`](https://npmjs.org/package/cli-highlight) library.
+ */
+class CliHighlightStep extends TaintTracking::SharedTaintStep {
+  override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(API::CallNode call |
+      call = API::moduleImport("cli-highlight").getMember("highlight").getACall()
+    |
+      pred = call.getArgument(0) and
+      succ = call
+    )
+  }
+}
+
+/**
+ * A step through the [`cli-color`](https://npmjs.org/package/cli-color) library.
+ */
+class CliColorStep extends TaintTracking::SharedTaintStep {
+  override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(API::CallNode call | call = API::moduleImport("cli-color").getAMember*().getACall() |
+      pred = call.getArgument(0) and
+      succ = call
+    )
+  }
+}
+
+/**
+ * A step through the [`slice-ansi`](https://npmjs.org/package/slice-ansi) library.
+ */
+class SliceAnsiStep extends TaintTracking::SharedTaintStep {
+  override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(API::CallNode call | call = API::moduleImport("slice-ansi").getACall() |
+      pred = call.getArgument(0) and
+      succ = call
+    )
+  }
+}
+
+/**
+ * A step through the [`kleur`](https://npmjs.org/package/kleur) library.
+ */
+class KleurStep extends TaintTracking::SharedTaintStep {
+  private API::Node kleurInstance() {
+    result = API::moduleImport("kleur")
+    or
+    result = kleurInstance().getAMember().getReturn()
+  }
+
+  override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(API::CallNode call | call = kleurInstance().getAMember().getACall() |
+      pred = call.getArgument(0) and
+      succ = call
+    )
+  }
+}
+
+/**
+ * A step through the [`chalk`](https://npmjs.org/package/chalk) library.
+ */
+class ChalkStep extends TaintTracking::SharedTaintStep {
+  override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(API::CallNode call | call = API::moduleImport("chalk").getAMember*().getACall() |
+      pred = call.getArgument(0) and
+      succ = call
+    )
+  }
+}
+
+/**
+ * A step through the [`strip-ansi`](https://npmjs.org/package/strip-ansi) library.
+ */
+class StripAnsiStep extends TaintTracking::SharedTaintStep {
+  override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(API::CallNode call | call = API::moduleImport("strip-ansi").getACall() |
+      pred = call.getArgument(0) and
+      succ = call
+    )
+  }
+}
--- a/javascript/ql/test/query-tests/Security/CWE-117/LogInjection.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-117/LogInjection.expected
@@ -22,6 +22,49 @@ nodes
 | logInjectionBad.js:30:23:30:49 | `[ERROR ... rror}"` |
 | logInjectionBad.js:30:23:30:49 | `[ERROR ... rror}"` |
 | logInjectionBad.js:30:42:30:46 | error |
+| logInjectionBad.js:46:9:46:36 | q |
+| logInjectionBad.js:46:13:46:36 | url.par ... , true) |
+| logInjectionBad.js:46:23:46:29 | req.url |
+| logInjectionBad.js:46:23:46:29 | req.url |
+| logInjectionBad.js:47:9:47:35 | username |
+| logInjectionBad.js:47:20:47:20 | q |
+| logInjectionBad.js:47:20:47:26 | q.query |
+| logInjectionBad.js:47:20:47:35 | q.query.username |
+| logInjectionBad.js:49:18:49:54 | ansiCol ... ername) |
+| logInjectionBad.js:49:18:49:54 | ansiCol ... ername) |
+| logInjectionBad.js:49:46:49:53 | username |
+| logInjectionBad.js:50:18:50:47 | colors. ... ername) |
+| logInjectionBad.js:50:18:50:47 | colors. ... ername) |
+| logInjectionBad.js:50:39:50:46 | username |
+| logInjectionBad.js:51:18:51:61 | wrapAns ... e), 20) |
+| logInjectionBad.js:51:18:51:61 | wrapAns ... e), 20) |
+| logInjectionBad.js:51:27:51:56 | colors. ... ername) |
+| logInjectionBad.js:51:48:51:55 | username |
+| logInjectionBad.js:52:17:52:47 | underli ... name))) |
+| logInjectionBad.js:52:17:52:47 | underli ... name))) |
+| logInjectionBad.js:52:27:52:46 | bold(blue(username)) |
+| logInjectionBad.js:52:32:52:45 | blue(username) |
+| logInjectionBad.js:52:37:52:44 | username |
+| logInjectionBad.js:53:17:53:76 | highlig ...  true}) |
+| logInjectionBad.js:53:17:53:76 | highlig ...  true}) |
+| logInjectionBad.js:53:27:53:34 | username |
+| logInjectionBad.js:54:17:54:51 | clc.red ... ername) |
+| logInjectionBad.js:54:17:54:51 | clc.red ... ername) |
+| logInjectionBad.js:54:43:54:50 | username |
+| logInjectionBad.js:55:17:55:65 | sliceAn ... 20, 30) |
+| logInjectionBad.js:55:17:55:65 | sliceAn ... 20, 30) |
+| logInjectionBad.js:55:27:55:56 | colors. ... ername) |
+| logInjectionBad.js:55:48:55:55 | username |
+| logInjectionBad.js:56:17:56:55 | kleur.b ... ername) |
+| logInjectionBad.js:56:17:56:55 | kleur.b ... ername) |
+| logInjectionBad.js:56:47:56:54 | username |
+| logInjectionBad.js:57:17:57:48 | chalk.u ... ername) |
+| logInjectionBad.js:57:17:57:48 | chalk.u ... ername) |
+| logInjectionBad.js:57:40:57:47 | username |
+| logInjectionBad.js:58:17:58:59 | stripAn ... rname)) |
+| logInjectionBad.js:58:17:58:59 | stripAn ... rname)) |
+| logInjectionBad.js:58:27:58:58 | chalk.u ... ername) |
+| logInjectionBad.js:58:50:58:57 | username |
 edges
 | logInjectionBad.js:19:9:19:36 | q | logInjectionBad.js:20:20:20:20 | q |
 | logInjectionBad.js:19:13:19:36 | url.par ... , true) | logInjectionBad.js:19:9:19:36 | q |
@@ -45,9 +88,61 @@ edges
 | logInjectionBad.js:29:14:29:18 | error | logInjectionBad.js:30:42:30:46 | error |
 | logInjectionBad.js:30:42:30:46 | error | logInjectionBad.js:30:23:30:49 | `[ERROR ... rror}"` |
 | logInjectionBad.js:30:42:30:46 | error | logInjectionBad.js:30:23:30:49 | `[ERROR ... rror}"` |
+| logInjectionBad.js:46:9:46:36 | q | logInjectionBad.js:47:20:47:20 | q |
+| logInjectionBad.js:46:13:46:36 | url.par ... , true) | logInjectionBad.js:46:9:46:36 | q |
+| logInjectionBad.js:46:23:46:29 | req.url | logInjectionBad.js:46:13:46:36 | url.par ... , true) |
+| logInjectionBad.js:46:23:46:29 | req.url | logInjectionBad.js:46:13:46:36 | url.par ... , true) |
+| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:49:46:49:53 | username |
+| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:50:39:50:46 | username |
+| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:51:48:51:55 | username |
+| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:52:37:52:44 | username |
+| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:53:27:53:34 | username |
+| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:54:43:54:50 | username |
+| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:55:48:55:55 | username |
+| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:56:47:56:54 | username |
+| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:57:40:57:47 | username |
+| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:58:50:58:57 | username |
+| logInjectionBad.js:47:20:47:20 | q | logInjectionBad.js:47:20:47:26 | q.query |
+| logInjectionBad.js:47:20:47:26 | q.query | logInjectionBad.js:47:20:47:35 | q.query.username |
+| logInjectionBad.js:47:20:47:35 | q.query.username | logInjectionBad.js:47:9:47:35 | username |
+| logInjectionBad.js:49:46:49:53 | username | logInjectionBad.js:49:18:49:54 | ansiCol ... ername) |
+| logInjectionBad.js:49:46:49:53 | username | logInjectionBad.js:49:18:49:54 | ansiCol ... ername) |
+| logInjectionBad.js:50:39:50:46 | username | logInjectionBad.js:50:18:50:47 | colors. ... ername) |
+| logInjectionBad.js:50:39:50:46 | username | logInjectionBad.js:50:18:50:47 | colors. ... ername) |
+| logInjectionBad.js:51:27:51:56 | colors. ... ername) | logInjectionBad.js:51:18:51:61 | wrapAns ... e), 20) |
+| logInjectionBad.js:51:27:51:56 | colors. ... ername) | logInjectionBad.js:51:18:51:61 | wrapAns ... e), 20) |
+| logInjectionBad.js:51:48:51:55 | username | logInjectionBad.js:51:27:51:56 | colors. ... ername) |
+| logInjectionBad.js:52:27:52:46 | bold(blue(username)) | logInjectionBad.js:52:17:52:47 | underli ... name))) |
+| logInjectionBad.js:52:27:52:46 | bold(blue(username)) | logInjectionBad.js:52:17:52:47 | underli ... name))) |
+| logInjectionBad.js:52:32:52:45 | blue(username) | logInjectionBad.js:52:27:52:46 | bold(blue(username)) |
+| logInjectionBad.js:52:37:52:44 | username | logInjectionBad.js:52:32:52:45 | blue(username) |
+| logInjectionBad.js:53:27:53:34 | username | logInjectionBad.js:53:17:53:76 | highlig ...  true}) |
+| logInjectionBad.js:53:27:53:34 | username | logInjectionBad.js:53:17:53:76 | highlig ...  true}) |
+| logInjectionBad.js:54:43:54:50 | username | logInjectionBad.js:54:17:54:51 | clc.red ... ername) |
+| logInjectionBad.js:54:43:54:50 | username | logInjectionBad.js:54:17:54:51 | clc.red ... ername) |
+| logInjectionBad.js:55:27:55:56 | colors. ... ername) | logInjectionBad.js:55:17:55:65 | sliceAn ... 20, 30) |
+| logInjectionBad.js:55:27:55:56 | colors. ... ername) | logInjectionBad.js:55:17:55:65 | sliceAn ... 20, 30) |
+| logInjectionBad.js:55:48:55:55 | username | logInjectionBad.js:55:27:55:56 | colors. ... ername) |
+| logInjectionBad.js:56:47:56:54 | username | logInjectionBad.js:56:17:56:55 | kleur.b ... ername) |
+| logInjectionBad.js:56:47:56:54 | username | logInjectionBad.js:56:17:56:55 | kleur.b ... ername) |
+| logInjectionBad.js:57:40:57:47 | username | logInjectionBad.js:57:17:57:48 | chalk.u ... ername) |
+| logInjectionBad.js:57:40:57:47 | username | logInjectionBad.js:57:17:57:48 | chalk.u ... ername) |
+| logInjectionBad.js:58:27:58:58 | chalk.u ... ername) | logInjectionBad.js:58:17:58:59 | stripAn ... rname)) |
+| logInjectionBad.js:58:27:58:58 | chalk.u ... ername) | logInjectionBad.js:58:17:58:59 | stripAn ... rname)) |
+| logInjectionBad.js:58:50:58:57 | username | logInjectionBad.js:58:27:58:58 | chalk.u ... ername) |
 #select
 | logInjectionBad.js:22:18:22:43 | `[INFO] ... rname}` | logInjectionBad.js:19:23:19:29 | req.url | logInjectionBad.js:22:18:22:43 | `[INFO] ... rname}` | $@ flows to log entry. | logInjectionBad.js:19:23:19:29 | req.url | User-provided value |
 | logInjectionBad.js:23:37:23:44 | username | logInjectionBad.js:19:23:19:29 | req.url | logInjectionBad.js:23:37:23:44 | username | $@ flows to log entry. | logInjectionBad.js:19:23:19:29 | req.url | User-provided value |
 | logInjectionBad.js:24:35:24:42 | username | logInjectionBad.js:19:23:19:29 | req.url | logInjectionBad.js:24:35:24:42 | username | $@ flows to log entry. | logInjectionBad.js:19:23:19:29 | req.url | User-provided value |
 | logInjectionBad.js:25:36:25:43 | username | logInjectionBad.js:19:23:19:29 | req.url | logInjectionBad.js:25:36:25:43 | username | $@ flows to log entry. | logInjectionBad.js:19:23:19:29 | req.url | User-provided value |
 | logInjectionBad.js:30:23:30:49 | `[ERROR ... rror}"` | logInjectionBad.js:19:23:19:29 | req.url | logInjectionBad.js:30:23:30:49 | `[ERROR ... rror}"` | $@ flows to log entry. | logInjectionBad.js:19:23:19:29 | req.url | User-provided value |
+| logInjectionBad.js:49:18:49:54 | ansiCol ... ername) | logInjectionBad.js:46:23:46:29 | req.url | logInjectionBad.js:49:18:49:54 | ansiCol ... ername) | $@ flows to log entry. | logInjectionBad.js:46:23:46:29 | req.url | User-provided value |
+| logInjectionBad.js:50:18:50:47 | colors. ... ername) | logInjectionBad.js:46:23:46:29 | req.url | logInjectionBad.js:50:18:50:47 | colors. ... ername) | $@ flows to log entry. | logInjectionBad.js:46:23:46:29 | req.url | User-provided value |
+| logInjectionBad.js:51:18:51:61 | wrapAns ... e), 20) | logInjectionBad.js:46:23:46:29 | req.url | logInjectionBad.js:51:18:51:61 | wrapAns ... e), 20) | $@ flows to log entry. | logInjectionBad.js:46:23:46:29 | req.url | User-provided value |
+| logInjectionBad.js:52:17:52:47 | underli ... name))) | logInjectionBad.js:46:23:46:29 | req.url | logInjectionBad.js:52:17:52:47 | underli ... name))) | $@ flows to log entry. | logInjectionBad.js:46:23:46:29 | req.url | User-provided value |
+| logInjectionBad.js:53:17:53:76 | highlig ...  true}) | logInjectionBad.js:46:23:46:29 | req.url | logInjectionBad.js:53:17:53:76 | highlig ...  true}) | $@ flows to log entry. | logInjectionBad.js:46:23:46:29 | req.url | User-provided value |
+| logInjectionBad.js:54:17:54:51 | clc.red ... ername) | logInjectionBad.js:46:23:46:29 | req.url | logInjectionBad.js:54:17:54:51 | clc.red ... ername) | $@ flows to log entry. | logInjectionBad.js:46:23:46:29 | req.url | User-provided value |
+| logInjectionBad.js:55:17:55:65 | sliceAn ... 20, 30) | logInjectionBad.js:46:23:46:29 | req.url | logInjectionBad.js:55:17:55:65 | sliceAn ... 20, 30) | $@ flows to log entry. | logInjectionBad.js:46:23:46:29 | req.url | User-provided value |
+| logInjectionBad.js:56:17:56:55 | kleur.b ... ername) | logInjectionBad.js:46:23:46:29 | req.url | logInjectionBad.js:56:17:56:55 | kleur.b ... ername) | $@ flows to log entry. | logInjectionBad.js:46:23:46:29 | req.url | User-provided value |
+| logInjectionBad.js:57:17:57:48 | chalk.u ... ername) | logInjectionBad.js:46:23:46:29 | req.url | logInjectionBad.js:57:17:57:48 | chalk.u ... ername) | $@ flows to log entry. | logInjectionBad.js:46:23:46:29 | req.url | User-provided value |
+| logInjectionBad.js:58:17:58:59 | stripAn ... rname)) | logInjectionBad.js:46:23:46:29 | req.url | logInjectionBad.js:58:17:58:59 | stripAn ... rname)) | $@ flows to log entry. | logInjectionBad.js:46:23:46:29 | req.url | User-provided value |
--- a/javascript/ql/test/query-tests/Security/CWE-117/logInjectionBad.js
+++ b/javascript/ql/test/query-tests/Security/CWE-117/logInjectionBad.js
@@ -29,4 +29,31 @@ const server = http.createServer((req, res) => {
    } catch (error) {
        console.error(`[ERROR] Error: "${error}"`); // NOT OK
    }
+});
+
+const ansiColors = require('ansi-colors');
+const colors = require('colors');
+import wrapAnsi from 'wrap-ansi';
+import { blue, bold, underline } from "colorette"
+const highlight = require('cli-highlight').highlight;
+var clc = require("cli-color");
+import sliceAnsi from 'slice-ansi';
+import kleur from 'kleur';
+const chalk = require('chalk');
+import stripAnsi from 'strip-ansi';
+
+const server2 = http.createServer((req, res) => {
+    let q = url.parse(req.url, true);
+    let username = q.query.username;
+
+    console.info(ansiColors.yellow.underline(username)); // NOT OK
+    console.info(colors.red.underline(username)); // NOT OK
+    console.info(wrapAnsi(colors.red.underline(username), 20)); // NOT OK
+    console.log(underline(bold(blue(username)))); // NOT OK
+    console.log(highlight(username, {language: 'sql', ignoreIllegals: true})); // NOT OK
+    console.log(clc.red.bgWhite.underline(username)); // NOT OK
+    console.log(sliceAnsi(colors.red.underline(username), 20, 30)); // NOT OK
+    console.log(kleur.blue().bold().underline(username)); // NOT OK
+    console.log(chalk.underline.bgBlue(username)); // NOT OK
+    console.log(stripAnsi(chalk.underline.bgBlue(username))); // NOT OK
 });