Skip to content

Commit c02c963

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#6139 from erik-krogh/colors
Approved by esbena
2 parents ffdc752 + dbc8b9c commit c02c963

File tree

4 files changed

+271
-0
lines changed

4 files changed

+271
-0
lines changed

‎javascript/change-notes/2021-06-22-colors.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
lgtm,codescanning
2+
* The dataflow libraries now model dataflow through console styling libraries.
3+
Affected packages are
4+
[ansi-colors](https://npmjs.com/package/ansi-colors),
5+
[colors](https://npmjs.com/package/colors),
6+
[wrap-ansi](https://npmjs.com/package/wrap-ansi),
7+
[colorette](https://npmjs.com/package/colorette),
8+
[cli-highlight](https://npmjs.com/package/cli-highlight),
9+
[cli-color](https://npmjs.com/package/cli-color),
10+
[slice-ansi](https://npmjs.com/package/slice-ansi),
11+
[kleur](https://npmjs.com/package/kleur),
12+
[chalk](https://npmjs.com/package/chalk),
13+
[strip-ansi](https://npmjs.com/package/strip-ansi)

‎javascript/ql/src/semmle/javascript/frameworks/Logging.qll

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,3 +201,139 @@ private class DebugLoggerCall extends LoggerCall, API::CallNode {
201201

202202
override DataFlow::Node getAMessageComponent() { result = getAnArgument() }
203203
}
204+
205+
/**
206+
* A step through the [`ansi-colors`](https://https://npmjs.org/package/ansi-colors) library.
207+
*/
208+
class AnsiColorsStep extends TaintTracking::SharedTaintStep {
209+
override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
210+
exists(API::CallNode call | call = API::moduleImport("ansi-colors").getAMember*().getACall() |
211+
pred = call.getArgument(0) and
212+
succ = call
213+
)
214+
}
215+
}
216+
217+
/**
218+
* A step through the [`colors`](https://npmjs.org/package/colors) library.
219+
* This step ignores the `String.prototype` modifying part of the `colors` library.
220+
*/
221+
class ColorsStep extends TaintTracking::SharedTaintStep {
222+
override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
223+
exists(API::CallNode call |
224+
call =
225+
API::moduleImport([
226+
"colors",
227+
// the `colors/safe` variant avoids modifying the prototype methods
228+
"colors/safe"
229+
]).getAMember*().getACall()
230+
|
231+
pred = call.getArgument(0) and
232+
succ = call
233+
)
234+
}
235+
}
236+
237+
/**
238+
* A step through the [`wrap-ansi`](https://npmjs.org/package/wrap-ansi) library.
239+
*/
240+
class WrapAnsiStep extends TaintTracking::SharedTaintStep {
241+
override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
242+
exists(API::CallNode call | call = API::moduleImport("wrap-ansi").getACall() |
243+
pred = call.getArgument(0) and
244+
succ = call
245+
)
246+
}
247+
}
248+
249+
/**
250+
* A step through the [`colorette`](https://npmjs.org/package/colorette) library.
251+
*/
252+
class ColoretteStep extends TaintTracking::SharedTaintStep {
253+
override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
254+
exists(API::CallNode call | call = API::moduleImport("colorette").getAMember().getACall() |
255+
pred = call.getArgument(0) and
256+
succ = call
257+
)
258+
}
259+
}
260+
261+
/**
262+
* A step through the [`cli-highlight`](https://npmjs.org/package/cli-highlight) library.
263+
*/
264+
class CliHighlightStep extends TaintTracking::SharedTaintStep {
265+
override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
266+
exists(API::CallNode call |
267+
call = API::moduleImport("cli-highlight").getMember("highlight").getACall()
268+
|
269+
pred = call.getArgument(0) and
270+
succ = call
271+
)
272+
}
273+
}
274+
275+
/**
276+
* A step through the [`cli-color`](https://npmjs.org/package/cli-color) library.
277+
*/
278+
class CliColorStep extends TaintTracking::SharedTaintStep {
279+
override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
280+
exists(API::CallNode call | call = API::moduleImport("cli-color").getAMember*().getACall() |
281+
pred = call.getArgument(0) and
282+
succ = call
283+
)
284+
}
285+
}
286+
287+
/**
288+
* A step through the [`slice-ansi`](https://npmjs.org/package/slice-ansi) library.
289+
*/
290+
class SliceAnsiStep extends TaintTracking::SharedTaintStep {
291+
override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
292+
exists(API::CallNode call | call = API::moduleImport("slice-ansi").getACall() |
293+
pred = call.getArgument(0) and
294+
succ = call
295+
)
296+
}
297+
}
298+
299+
/**
300+
* A step through the [`kleur`](https://npmjs.org/package/kleur) library.
301+
*/
302+
class KleurStep extends TaintTracking::SharedTaintStep {
303+
private API::Node kleurInstance() {
304+
result = API::moduleImport("kleur")
305+
or
306+
result = kleurInstance().getAMember().getReturn()
307+
}
308+
309+
override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
310+
exists(API::CallNode call | call = kleurInstance().getAMember().getACall() |
311+
pred = call.getArgument(0) and
312+
succ = call
313+
)
314+
}
315+
}
316+
317+
/**
318+
* A step through the [`chalk`](https://npmjs.org/package/chalk) library.
319+
*/
320+
class ChalkStep extends TaintTracking::SharedTaintStep {
321+
override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
322+
exists(API::CallNode call | call = API::moduleImport("chalk").getAMember*().getACall() |
323+
pred = call.getArgument(0) and
324+
succ = call
325+
)
326+
}
327+
}
328+
329+
/**
330+
* A step through the [`strip-ansi`](https://npmjs.org/package/strip-ansi) library.
331+
*/
332+
class StripAnsiStep extends TaintTracking::SharedTaintStep {
333+
override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
334+
exists(API::CallNode call | call = API::moduleImport("strip-ansi").getACall() |
335+
pred = call.getArgument(0) and
336+
succ = call
337+
)
338+
}
339+
}

‎javascript/ql/test/query-tests/Security/CWE-117/LogInjection.expected

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,49 @@ nodes
2222
| logInjectionBad.js:30:23:30:49 | `[ERROR ... rror}"` |
2323
| logInjectionBad.js:30:23:30:49 | `[ERROR ... rror}"` |
2424
| logInjectionBad.js:30:42:30:46 | error |
25+
| logInjectionBad.js:46:9:46:36 | q |
26+
| logInjectionBad.js:46:13:46:36 | url.par ... , true) |
27+
| logInjectionBad.js:46:23:46:29 | req.url |
28+
| logInjectionBad.js:46:23:46:29 | req.url |
29+
| logInjectionBad.js:47:9:47:35 | username |
30+
| logInjectionBad.js:47:20:47:20 | q |
31+
| logInjectionBad.js:47:20:47:26 | q.query |
32+
| logInjectionBad.js:47:20:47:35 | q.query.username |
33+
| logInjectionBad.js:49:18:49:54 | ansiCol ... ername) |
34+
| logInjectionBad.js:49:18:49:54 | ansiCol ... ername) |
35+
| logInjectionBad.js:49:46:49:53 | username |
36+
| logInjectionBad.js:50:18:50:47 | colors. ... ername) |
37+
| logInjectionBad.js:50:18:50:47 | colors. ... ername) |
38+
| logInjectionBad.js:50:39:50:46 | username |
39+
| logInjectionBad.js:51:18:51:61 | wrapAns ... e), 20) |
40+
| logInjectionBad.js:51:18:51:61 | wrapAns ... e), 20) |
41+
| logInjectionBad.js:51:27:51:56 | colors. ... ername) |
42+
| logInjectionBad.js:51:48:51:55 | username |
43+
| logInjectionBad.js:52:17:52:47 | underli ... name))) |
44+
| logInjectionBad.js:52:17:52:47 | underli ... name))) |
45+
| logInjectionBad.js:52:27:52:46 | bold(blue(username)) |
46+
| logInjectionBad.js:52:32:52:45 | blue(username) |
47+
| logInjectionBad.js:52:37:52:44 | username |
48+
| logInjectionBad.js:53:17:53:76 | highlig ... true}) |
49+
| logInjectionBad.js:53:17:53:76 | highlig ... true}) |
50+
| logInjectionBad.js:53:27:53:34 | username |
51+
| logInjectionBad.js:54:17:54:51 | clc.red ... ername) |
52+
| logInjectionBad.js:54:17:54:51 | clc.red ... ername) |
53+
| logInjectionBad.js:54:43:54:50 | username |
54+
| logInjectionBad.js:55:17:55:65 | sliceAn ... 20, 30) |
55+
| logInjectionBad.js:55:17:55:65 | sliceAn ... 20, 30) |
56+
| logInjectionBad.js:55:27:55:56 | colors. ... ername) |
57+
| logInjectionBad.js:55:48:55:55 | username |
58+
| logInjectionBad.js:56:17:56:55 | kleur.b ... ername) |
59+
| logInjectionBad.js:56:17:56:55 | kleur.b ... ername) |
60+
| logInjectionBad.js:56:47:56:54 | username |
61+
| logInjectionBad.js:57:17:57:48 | chalk.u ... ername) |
62+
| logInjectionBad.js:57:17:57:48 | chalk.u ... ername) |
63+
| logInjectionBad.js:57:40:57:47 | username |
64+
| logInjectionBad.js:58:17:58:59 | stripAn ... rname)) |
65+
| logInjectionBad.js:58:17:58:59 | stripAn ... rname)) |
66+
| logInjectionBad.js:58:27:58:58 | chalk.u ... ername) |
67+
| logInjectionBad.js:58:50:58:57 | username |
2568
edges
2669
| logInjectionBad.js:19:9:19:36 | q | logInjectionBad.js:20:20:20:20 | q |
2770
| logInjectionBad.js:19:13:19:36 | url.par ... , true) | logInjectionBad.js:19:9:19:36 | q |
@@ -45,9 +88,61 @@ edges
4588
| logInjectionBad.js:29:14:29:18 | error | logInjectionBad.js:30:42:30:46 | error |
4689
| logInjectionBad.js:30:42:30:46 | error | logInjectionBad.js:30:23:30:49 | `[ERROR ... rror}"` |
4790
| logInjectionBad.js:30:42:30:46 | error | logInjectionBad.js:30:23:30:49 | `[ERROR ... rror}"` |
91+
| logInjectionBad.js:46:9:46:36 | q | logInjectionBad.js:47:20:47:20 | q |
92+
| logInjectionBad.js:46:13:46:36 | url.par ... , true) | logInjectionBad.js:46:9:46:36 | q |
93+
| logInjectionBad.js:46:23:46:29 | req.url | logInjectionBad.js:46:13:46:36 | url.par ... , true) |
94+
| logInjectionBad.js:46:23:46:29 | req.url | logInjectionBad.js:46:13:46:36 | url.par ... , true) |
95+
| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:49:46:49:53 | username |
96+
| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:50:39:50:46 | username |
97+
| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:51:48:51:55 | username |
98+
| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:52:37:52:44 | username |
99+
| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:53:27:53:34 | username |
100+
| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:54:43:54:50 | username |
101+
| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:55:48:55:55 | username |
102+
| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:56:47:56:54 | username |
103+
| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:57:40:57:47 | username |
104+
| logInjectionBad.js:47:9:47:35 | username | logInjectionBad.js:58:50:58:57 | username |
105+
| logInjectionBad.js:47:20:47:20 | q | logInjectionBad.js:47:20:47:26 | q.query |
106+
| logInjectionBad.js:47:20:47:26 | q.query | logInjectionBad.js:47:20:47:35 | q.query.username |
107+
| logInjectionBad.js:47:20:47:35 | q.query.username | logInjectionBad.js:47:9:47:35 | username |
108+
| logInjectionBad.js:49:46:49:53 | username | logInjectionBad.js:49:18:49:54 | ansiCol ... ername) |
109+
| logInjectionBad.js:49:46:49:53 | username | logInjectionBad.js:49:18:49:54 | ansiCol ... ername) |
110+
| logInjectionBad.js:50:39:50:46 | username | logInjectionBad.js:50:18:50:47 | colors. ... ername) |
111+
| logInjectionBad.js:50:39:50:46 | username | logInjectionBad.js:50:18:50:47 | colors. ... ername) |
112+
| logInjectionBad.js:51:27:51:56 | colors. ... ername) | logInjectionBad.js:51:18:51:61 | wrapAns ... e), 20) |
113+
| logInjectionBad.js:51:27:51:56 | colors. ... ername) | logInjectionBad.js:51:18:51:61 | wrapAns ... e), 20) |
114+
| logInjectionBad.js:51:48:51:55 | username | logInjectionBad.js:51:27:51:56 | colors. ... ername) |
115+
| logInjectionBad.js:52:27:52:46 | bold(blue(username)) | logInjectionBad.js:52:17:52:47 | underli ... name))) |
116+
| logInjectionBad.js:52:27:52:46 | bold(blue(username)) | logInjectionBad.js:52:17:52:47 | underli ... name))) |
117+
| logInjectionBad.js:52:32:52:45 | blue(username) | logInjectionBad.js:52:27:52:46 | bold(blue(username)) |
118+
| logInjectionBad.js:52:37:52:44 | username | logInjectionBad.js:52:32:52:45 | blue(username) |
119+
| logInjectionBad.js:53:27:53:34 | username | logInjectionBad.js:53:17:53:76 | highlig ... true}) |
120+
| logInjectionBad.js:53:27:53:34 | username | logInjectionBad.js:53:17:53:76 | highlig ... true}) |
121+
| logInjectionBad.js:54:43:54:50 | username | logInjectionBad.js:54:17:54:51 | clc.red ... ername) |
122+
| logInjectionBad.js:54:43:54:50 | username | logInjectionBad.js:54:17:54:51 | clc.red ... ername) |
123+
| logInjectionBad.js:55:27:55:56 | colors. ... ername) | logInjectionBad.js:55:17:55:65 | sliceAn ... 20, 30) |
124+
| logInjectionBad.js:55:27:55:56 | colors. ... ername) | logInjectionBad.js:55:17:55:65 | sliceAn ... 20, 30) |
125+
| logInjectionBad.js:55:48:55:55 | username | logInjectionBad.js:55:27:55:56 | colors. ... ername) |
126+
| logInjectionBad.js:56:47:56:54 | username | logInjectionBad.js:56:17:56:55 | kleur.b ... ername) |
127+
| logInjectionBad.js:56:47:56:54 | username | logInjectionBad.js:56:17:56:55 | kleur.b ... ername) |
128+
| logInjectionBad.js:57:40:57:47 | username | logInjectionBad.js:57:17:57:48 | chalk.u ... ername) |
129+
| logInjectionBad.js:57:40:57:47 | username | logInjectionBad.js:57:17:57:48 | chalk.u ... ername) |
130+
| logInjectionBad.js:58:27:58:58 | chalk.u ... ername) | logInjectionBad.js:58:17:58:59 | stripAn ... rname)) |
131+
| logInjectionBad.js:58:27:58:58 | chalk.u ... ername) | logInjectionBad.js:58:17:58:59 | stripAn ... rname)) |
132+
| logInjectionBad.js:58:50:58:57 | username | logInjectionBad.js:58:27:58:58 | chalk.u ... ername) |
48133
#select
49134
| 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 |
50135
| 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 |
51136
| 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 |
52137
| 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 |
53138
| 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 |
139+
| 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 |
140+
| 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 |
141+
| 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 |
142+
| 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 |
143+
| 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 |
144+
| 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 |
145+
| 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 |
146+
| 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 |
147+
| 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 |
148+
| 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 |

‎javascript/ql/test/query-tests/Security/CWE-117/logInjectionBad.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,31 @@ const server = http.createServer((req, res) => {
2929
} catch (error) {
3030
console.error(`[ERROR] Error: "${error}"`); // NOT OK
3131
}
32+
});
33+
34+
const ansiColors = require('ansi-colors');
35+
const colors = require('colors');
36+
import wrapAnsi from 'wrap-ansi';
37+
import { blue, bold, underline } from "colorette"
38+
const highlight = require('cli-highlight').highlight;
39+
var clc = require("cli-color");
40+
import sliceAnsi from 'slice-ansi';
41+
import kleur from 'kleur';
42+
const chalk = require('chalk');
43+
import stripAnsi from 'strip-ansi';
44+
45+
const server2 = http.createServer((req, res) => {
46+
let q = url.parse(req.url, true);
47+
let username = q.query.username;
48+
49+
console.info(ansiColors.yellow.underline(username)); // NOT OK
50+
console.info(colors.red.underline(username)); // NOT OK
51+
console.info(wrapAnsi(colors.red.underline(username), 20)); // NOT OK
52+
console.log(underline(bold(blue(username)))); // NOT OK
53+
console.log(highlight(username, {language: 'sql', ignoreIllegals: true})); // NOT OK
54+
console.log(clc.red.bgWhite.underline(username)); // NOT OK
55+
console.log(sliceAnsi(colors.red.underline(username), 20, 30)); // NOT OK
56+
console.log(kleur.blue().bold().underline(username)); // NOT OK
57+
console.log(chalk.underline.bgBlue(username)); // NOT OK
58+
console.log(stripAnsi(chalk.underline.bgBlue(username))); // NOT OK
3259
});

0 commit comments

Comments
 (0)