Skip to content

Commit f9d62ad

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#4567 from asgerf/js/date-functions
Approved by erik-krogh
2 parents 6113985 + 1e45bc7 commit f9d62ad

File tree

6 files changed

+223
-0
lines changed

6 files changed

+223
-0
lines changed

‎javascript/change-notes/2020-11-06-date-functions.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
lgtm,codescanning
2+
* The security queries now track taint through the format string of a date-formatting operation.
3+
Affected packages are
4+
[moment](https://npmjs.com/package/moment),
5+
[moment-timezone](https://npmjs.com/package/moment-timezone),
6+
[date-fns](https://npmjs.com/package/date-fns), and
7+
[dateformat](https://npmjs.com/package/dateformat).

‎javascript/ql/src/javascript.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ import semmle.javascript.frameworks.ClosureLibrary
7979
import semmle.javascript.frameworks.CookieLibraries
8080
import semmle.javascript.frameworks.Credentials
8181
import semmle.javascript.frameworks.CryptoLibraries
82+
import semmle.javascript.frameworks.DateFunctions
8283
import semmle.javascript.frameworks.DigitalOcean
8384
import semmle.javascript.frameworks.Electron
8485
import semmle.javascript.frameworks.EventEmitter

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

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/** Provides taint steps modeling flow through date-manipulation libraries. */
2+
3+
private import javascript
4+
5+
private module DateFns {
6+
private API::Node formatFunction() {
7+
result = API::moduleImport(["date-fns", "date-fns/esm"]).getMember(["format", "lightFormat"])
8+
or
9+
result =
10+
API::moduleImport(["date-fns/format", "date-fns/lightFormat", "date-fns/esm/format",
11+
"date-fns/esm/lightFormat"])
12+
}
13+
14+
private API::Node curriedFormatFunction() {
15+
result =
16+
API::moduleImport(["date-fns/fp", "date-fns/esm/fp"]).getMember(["format", "lightFormat"])
17+
or
18+
result =
19+
API::moduleImport(["date-fns/fp/format", "date-fns/fp/lightFormat", "date-fns/esm/fp/format",
20+
"date-fns/esm/fp/lightFormat"])
21+
}
22+
23+
/**
24+
* Taint step of form: `f -> format(date, f)`
25+
*
26+
* A format string can use single-quotes to include mostly arbitrary text.
27+
*/
28+
private class FormatStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
29+
FormatStep() { this = formatFunction().getACall() }
30+
31+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
32+
pred = getArgument(1) and
33+
succ = this
34+
}
35+
}
36+
37+
/**
38+
* Taint step of form: `f -> format(f)(date)`
39+
*/
40+
private class CurriedFormatStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
41+
CurriedFormatStep() { this = curriedFormatFunction().getACall() }
42+
43+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
44+
pred = getArgument(0) and
45+
succ = getACall()
46+
}
47+
}
48+
}
49+
50+
private module Moment {
51+
/** Gets a reference to a `moment` object. */
52+
private API::Node moment() {
53+
result = API::moduleImport(["moment", "moment-timezone"])
54+
or
55+
result = moment().getReturn()
56+
or
57+
result = moment().getAMember()
58+
}
59+
60+
/**
61+
* Taint step of form: `f -> momentObj.format(f)`
62+
*
63+
* The format string can use backslash-escaping to include mostly arbitrary text.
64+
*/
65+
private class MomentFormatStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
66+
MomentFormatStep() { this = moment().getMember("format").getACall() }
67+
68+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
69+
pred = getArgument(0) and
70+
succ = this
71+
}
72+
}
73+
}
74+
75+
private module DateFormat {
76+
/**
77+
* Taint step of form: `x -> dateformat(..., x)`
78+
*
79+
* The format string can use single-quotes to include mostly arbitrary text.
80+
*/
81+
private class DateFormatStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
82+
DateFormatStep() { this = DataFlow::moduleImport("dateformat").getACall() }
83+
84+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
85+
pred = getArgument(1) and
86+
succ = this
87+
}
88+
}
89+
}

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,32 @@ nodes
8989
| classnames.js:15:47:15:63 | clsx(window.name) |
9090
| classnames.js:15:52:15:62 | window.name |
9191
| classnames.js:15:52:15:62 | window.name |
92+
| dates.js:9:9:9:69 | taint |
93+
| dates.js:9:17:9:69 | decodeU ... ing(1)) |
94+
| dates.js:9:36:9:50 | window.___location |
95+
| dates.js:9:36:9:50 | window.___location |
96+
| dates.js:9:36:9:55 | window.___location.hash |
97+
| dates.js:9:36:9:68 | window. ... ring(1) |
98+
| dates.js:11:31:11:70 | `Time i ... aint)}` |
99+
| dates.js:11:31:11:70 | `Time i ... aint)}` |
100+
| dates.js:11:42:11:68 | dateFns ... taint) |
101+
| dates.js:11:63:11:67 | taint |
102+
| dates.js:12:31:12:73 | `Time i ... aint)}` |
103+
| dates.js:12:31:12:73 | `Time i ... aint)}` |
104+
| dates.js:12:42:12:71 | dateFns ... taint) |
105+
| dates.js:12:66:12:70 | taint |
106+
| dates.js:13:31:13:72 | `Time i ... time)}` |
107+
| dates.js:13:31:13:72 | `Time i ... time)}` |
108+
| dates.js:13:42:13:70 | dateFns ... )(time) |
109+
| dates.js:13:59:13:63 | taint |
110+
| dates.js:16:31:16:69 | `Time i ... aint)}` |
111+
| dates.js:16:31:16:69 | `Time i ... aint)}` |
112+
| dates.js:16:42:16:67 | moment( ... (taint) |
113+
| dates.js:16:62:16:66 | taint |
114+
| dates.js:18:31:18:66 | `Time i ... aint)}` |
115+
| dates.js:18:31:18:66 | `Time i ... aint)}` |
116+
| dates.js:18:42:18:64 | datefor ... taint) |
117+
| dates.js:18:59:18:63 | taint |
92118
| express.js:7:15:7:33 | req.param("wobble") |
93119
| express.js:7:15:7:33 | req.param("wobble") |
94120
| express.js:7:15:7:33 | req.param("wobble") |
@@ -688,6 +714,31 @@ edges
688714
| classnames.js:15:47:15:63 | clsx(window.name) | classnames.js:15:31:15:78 | `<span ... <span>` |
689715
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
690716
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
717+
| dates.js:9:9:9:69 | taint | dates.js:11:63:11:67 | taint |
718+
| dates.js:9:9:9:69 | taint | dates.js:12:66:12:70 | taint |
719+
| dates.js:9:9:9:69 | taint | dates.js:13:59:13:63 | taint |
720+
| dates.js:9:9:9:69 | taint | dates.js:16:62:16:66 | taint |
721+
| dates.js:9:9:9:69 | taint | dates.js:18:59:18:63 | taint |
722+
| dates.js:9:17:9:69 | decodeU ... ing(1)) | dates.js:9:9:9:69 | taint |
723+
| dates.js:9:36:9:50 | window.___location | dates.js:9:36:9:55 | window.___location.hash |
724+
| dates.js:9:36:9:50 | window.___location | dates.js:9:36:9:55 | window.___location.hash |
725+
| dates.js:9:36:9:55 | window.___location.hash | dates.js:9:36:9:68 | window. ... ring(1) |
726+
| dates.js:9:36:9:68 | window. ... ring(1) | dates.js:9:17:9:69 | decodeU ... ing(1)) |
727+
| dates.js:11:42:11:68 | dateFns ... taint) | dates.js:11:31:11:70 | `Time i ... aint)}` |
728+
| dates.js:11:42:11:68 | dateFns ... taint) | dates.js:11:31:11:70 | `Time i ... aint)}` |
729+
| dates.js:11:63:11:67 | taint | dates.js:11:42:11:68 | dateFns ... taint) |
730+
| dates.js:12:42:12:71 | dateFns ... taint) | dates.js:12:31:12:73 | `Time i ... aint)}` |
731+
| dates.js:12:42:12:71 | dateFns ... taint) | dates.js:12:31:12:73 | `Time i ... aint)}` |
732+
| dates.js:12:66:12:70 | taint | dates.js:12:42:12:71 | dateFns ... taint) |
733+
| dates.js:13:42:13:70 | dateFns ... )(time) | dates.js:13:31:13:72 | `Time i ... time)}` |
734+
| dates.js:13:42:13:70 | dateFns ... )(time) | dates.js:13:31:13:72 | `Time i ... time)}` |
735+
| dates.js:13:59:13:63 | taint | dates.js:13:42:13:70 | dateFns ... )(time) |
736+
| dates.js:16:42:16:67 | moment( ... (taint) | dates.js:16:31:16:69 | `Time i ... aint)}` |
737+
| dates.js:16:42:16:67 | moment( ... (taint) | dates.js:16:31:16:69 | `Time i ... aint)}` |
738+
| dates.js:16:62:16:66 | taint | dates.js:16:42:16:67 | moment( ... (taint) |
739+
| dates.js:18:42:18:64 | datefor ... taint) | dates.js:18:31:18:66 | `Time i ... aint)}` |
740+
| dates.js:18:42:18:64 | datefor ... taint) | dates.js:18:31:18:66 | `Time i ... aint)}` |
741+
| dates.js:18:59:18:63 | taint | dates.js:18:42:18:64 | datefor ... taint) |
691742
| express.js:7:15:7:33 | req.param("wobble") | express.js:7:15:7:33 | req.param("wobble") |
692743
| jquery.js:2:7:2:40 | tainted | jquery.js:7:20:7:26 | tainted |
693744
| jquery.js:2:7:2:40 | tainted | jquery.js:8:28:8:34 | tainted |
@@ -1179,6 +1230,11 @@ edges
11791230
| classnames.js:11:31:11:79 | `<span ... <span>` | classnames.js:10:45:10:55 | window.name | classnames.js:11:31:11:79 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:10:45:10:55 | window.name | user-provided value |
11801231
| classnames.js:13:31:13:83 | `<span ... <span>` | classnames.js:13:57:13:67 | window.name | classnames.js:13:31:13:83 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:13:57:13:67 | window.name | user-provided value |
11811232
| classnames.js:15:31:15:78 | `<span ... <span>` | classnames.js:15:52:15:62 | window.name | classnames.js:15:31:15:78 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:15:52:15:62 | window.name | user-provided value |
1233+
| dates.js:11:31:11:70 | `Time i ... aint)}` | dates.js:9:36:9:50 | window.___location | dates.js:11:31:11:70 | `Time i ... aint)}` | Cross-site scripting vulnerability due to $@. | dates.js:9:36:9:50 | window.___location | user-provided value |
1234+
| dates.js:12:31:12:73 | `Time i ... aint)}` | dates.js:9:36:9:50 | window.___location | dates.js:12:31:12:73 | `Time i ... aint)}` | Cross-site scripting vulnerability due to $@. | dates.js:9:36:9:50 | window.___location | user-provided value |
1235+
| dates.js:13:31:13:72 | `Time i ... time)}` | dates.js:9:36:9:50 | window.___location | dates.js:13:31:13:72 | `Time i ... time)}` | Cross-site scripting vulnerability due to $@. | dates.js:9:36:9:50 | window.___location | user-provided value |
1236+
| dates.js:16:31:16:69 | `Time i ... aint)}` | dates.js:9:36:9:50 | window.___location | dates.js:16:31:16:69 | `Time i ... aint)}` | Cross-site scripting vulnerability due to $@. | dates.js:9:36:9:50 | window.___location | user-provided value |
1237+
| dates.js:18:31:18:66 | `Time i ... aint)}` | dates.js:9:36:9:50 | window.___location | dates.js:18:31:18:66 | `Time i ... aint)}` | Cross-site scripting vulnerability due to $@. | dates.js:9:36:9:50 | window.___location | user-provided value |
11821238
| express.js:7:15:7:33 | req.param("wobble") | express.js:7:15:7:33 | req.param("wobble") | express.js:7:15:7:33 | req.param("wobble") | Cross-site scripting vulnerability due to $@. | express.js:7:15:7:33 | req.param("wobble") | user-provided value |
11831239
| jquery.js:7:5:7:34 | "<div i ... + "\\">" | jquery.js:2:17:2:40 | documen ... .search | jquery.js:7:5:7:34 | "<div i ... + "\\">" | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:40 | documen ... .search | user-provided value |
11841240
| jquery.js:8:18:8:34 | "XSS: " + tainted | jquery.js:2:17:2:33 | document.___location | jquery.js:8:18:8:34 | "XSS: " + tainted | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:33 | document.___location | user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,32 @@ nodes
8989
| classnames.js:15:47:15:63 | clsx(window.name) |
9090
| classnames.js:15:52:15:62 | window.name |
9191
| classnames.js:15:52:15:62 | window.name |
92+
| dates.js:9:9:9:69 | taint |
93+
| dates.js:9:17:9:69 | decodeU ... ing(1)) |
94+
| dates.js:9:36:9:50 | window.___location |
95+
| dates.js:9:36:9:50 | window.___location |
96+
| dates.js:9:36:9:55 | window.___location.hash |
97+
| dates.js:9:36:9:68 | window. ... ring(1) |
98+
| dates.js:11:31:11:70 | `Time i ... aint)}` |
99+
| dates.js:11:31:11:70 | `Time i ... aint)}` |
100+
| dates.js:11:42:11:68 | dateFns ... taint) |
101+
| dates.js:11:63:11:67 | taint |
102+
| dates.js:12:31:12:73 | `Time i ... aint)}` |
103+
| dates.js:12:31:12:73 | `Time i ... aint)}` |
104+
| dates.js:12:42:12:71 | dateFns ... taint) |
105+
| dates.js:12:66:12:70 | taint |
106+
| dates.js:13:31:13:72 | `Time i ... time)}` |
107+
| dates.js:13:31:13:72 | `Time i ... time)}` |
108+
| dates.js:13:42:13:70 | dateFns ... )(time) |
109+
| dates.js:13:59:13:63 | taint |
110+
| dates.js:16:31:16:69 | `Time i ... aint)}` |
111+
| dates.js:16:31:16:69 | `Time i ... aint)}` |
112+
| dates.js:16:42:16:67 | moment( ... (taint) |
113+
| dates.js:16:62:16:66 | taint |
114+
| dates.js:18:31:18:66 | `Time i ... aint)}` |
115+
| dates.js:18:31:18:66 | `Time i ... aint)}` |
116+
| dates.js:18:42:18:64 | datefor ... taint) |
117+
| dates.js:18:59:18:63 | taint |
92118
| express.js:7:15:7:33 | req.param("wobble") |
93119
| express.js:7:15:7:33 | req.param("wobble") |
94120
| express.js:7:15:7:33 | req.param("wobble") |
@@ -692,6 +718,31 @@ edges
692718
| classnames.js:15:47:15:63 | clsx(window.name) | classnames.js:15:31:15:78 | `<span ... <span>` |
693719
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
694720
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
721+
| dates.js:9:9:9:69 | taint | dates.js:11:63:11:67 | taint |
722+
| dates.js:9:9:9:69 | taint | dates.js:12:66:12:70 | taint |
723+
| dates.js:9:9:9:69 | taint | dates.js:13:59:13:63 | taint |
724+
| dates.js:9:9:9:69 | taint | dates.js:16:62:16:66 | taint |
725+
| dates.js:9:9:9:69 | taint | dates.js:18:59:18:63 | taint |
726+
| dates.js:9:17:9:69 | decodeU ... ing(1)) | dates.js:9:9:9:69 | taint |
727+
| dates.js:9:36:9:50 | window.___location | dates.js:9:36:9:55 | window.___location.hash |
728+
| dates.js:9:36:9:50 | window.___location | dates.js:9:36:9:55 | window.___location.hash |
729+
| dates.js:9:36:9:55 | window.___location.hash | dates.js:9:36:9:68 | window. ... ring(1) |
730+
| dates.js:9:36:9:68 | window. ... ring(1) | dates.js:9:17:9:69 | decodeU ... ing(1)) |
731+
| dates.js:11:42:11:68 | dateFns ... taint) | dates.js:11:31:11:70 | `Time i ... aint)}` |
732+
| dates.js:11:42:11:68 | dateFns ... taint) | dates.js:11:31:11:70 | `Time i ... aint)}` |
733+
| dates.js:11:63:11:67 | taint | dates.js:11:42:11:68 | dateFns ... taint) |
734+
| dates.js:12:42:12:71 | dateFns ... taint) | dates.js:12:31:12:73 | `Time i ... aint)}` |
735+
| dates.js:12:42:12:71 | dateFns ... taint) | dates.js:12:31:12:73 | `Time i ... aint)}` |
736+
| dates.js:12:66:12:70 | taint | dates.js:12:42:12:71 | dateFns ... taint) |
737+
| dates.js:13:42:13:70 | dateFns ... )(time) | dates.js:13:31:13:72 | `Time i ... time)}` |
738+
| dates.js:13:42:13:70 | dateFns ... )(time) | dates.js:13:31:13:72 | `Time i ... time)}` |
739+
| dates.js:13:59:13:63 | taint | dates.js:13:42:13:70 | dateFns ... )(time) |
740+
| dates.js:16:42:16:67 | moment( ... (taint) | dates.js:16:31:16:69 | `Time i ... aint)}` |
741+
| dates.js:16:42:16:67 | moment( ... (taint) | dates.js:16:31:16:69 | `Time i ... aint)}` |
742+
| dates.js:16:62:16:66 | taint | dates.js:16:42:16:67 | moment( ... (taint) |
743+
| dates.js:18:42:18:64 | datefor ... taint) | dates.js:18:31:18:66 | `Time i ... aint)}` |
744+
| dates.js:18:42:18:64 | datefor ... taint) | dates.js:18:31:18:66 | `Time i ... aint)}` |
745+
| dates.js:18:59:18:63 | taint | dates.js:18:42:18:64 | datefor ... taint) |
695746
| express.js:7:15:7:33 | req.param("wobble") | express.js:7:15:7:33 | req.param("wobble") |
696747
| jquery.js:2:7:2:40 | tainted | jquery.js:7:20:7:26 | tainted |
697748
| jquery.js:2:7:2:40 | tainted | jquery.js:8:28:8:34 | tainted |

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/dates.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import dateFns from 'date-fns';
2+
import dateFnsFp from 'date-fns/fp';
3+
import dateFnsEsm from 'date-fns/esm';
4+
import moment from 'moment';
5+
import dateformat from 'dateformat';
6+
7+
function main() {
8+
let time = new Date();
9+
let taint = decodeURIComponent(window.___location.hash.substring(1));
10+
11+
document.body.innerHTML = `Time is ${dateFns.format(time, taint)}`; // NOT OK
12+
document.body.innerHTML = `Time is ${dateFnsEsm.format(time, taint)}`; // NOT OK
13+
document.body.innerHTML = `Time is ${dateFnsFp.format(taint)(time)}`; // NOT OK
14+
document.body.innerHTML = `Time is ${dateFns.format(taint, time)}`; // OK - time arg is safe
15+
document.body.innerHTML = `Time is ${dateFnsFp.format(time)(taint)}`; // OK - time arg is safe
16+
document.body.innerHTML = `Time is ${moment(time).format(taint)}`; // NOT OK
17+
document.body.innerHTML = `Time is ${moment(taint).format()}`; // OK
18+
document.body.innerHTML = `Time is ${dateformat(time, taint)}`; // NOT OK
19+
}

0 commit comments

Comments
 (0)