hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-29 10:45:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #10670 from tyage/property-stringify

Browse Source 
JS: Improve detection of XSS when JSON.stringify()
This commit is contained in:
Asger F
2022-10-10 18:16:09 +02:00
committed by GitHub
parent 5756a33604 b1a165ee98
commit 9bbbece8a7
5 changed files with 135 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll
View File

@@ -711,13 +711,31 @@ module TaintTracking {
}
}
/**
* Gets a local source of any part of the input to the given stringification `call`.
*/
pragma[nomagic]
private DataFlow::Node getAJsonLocalInput(JsonStringifyCall call) {
result = call.getInput()
or
exists(DataFlow::SourceNode source |
source = pragma[only_bind_out](getAJsonLocalInput(call)).getALocalSource()
|
result = source.getAPropertyWrite().getRhs()
or
result = source.(DataFlow::ObjectLiteralNode).getASpreadProperty()
or
result = source.(DataFlow::ArrayCreationNode).getASpreadArgument()
)
}
/**
* A taint propagating data flow edge arising from JSON unparsing.
*/
private class JsonStringifyTaintStep extends SharedTaintStep {
override predicate serializeStep(DataFlow::Node pred, DataFlow::Node succ) {
exists(JsonStringifyCall call |
pred = call.getArgument(0) and
pred = getAJsonLocalInput(call) and
succ = call
)
}

4
javascript/ql/src/change-notes/2022-10-04-json-stringify-improvement.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Improved taint tracking through `JSON.stringify` in cases where a tainted value is stored somewhere in the input object.

36
javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected
View File

@@ -431,6 +431,22 @@ nodes
| jquery.js:34:5:34:25 | '<b>' + ... '</b>' |
| jquery.js:34:5:34:25 | '<b>' + ... '</b>' |
| jquery.js:34:13:34:16 | hash |
| json-stringify.jsx:5:9:5:36 | locale |
| json-stringify.jsx:5:9:5:36 | locale |
| json-stringify.jsx:5:18:5:36 | req.param("locale") |
| json-stringify.jsx:5:18:5:36 | req.param("locale") |
| json-stringify.jsx:5:18:5:36 | req.param("locale") |
| json-stringify.jsx:11:16:11:58 | `https: ... ocale}` |
| json-stringify.jsx:11:51:11:56 | locale |
| json-stringify.jsx:19:16:19:63 | `https: ... ocale}` |
| json-stringify.jsx:19:56:19:61 | locale |
| json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
| json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
| json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
| json-stringify.jsx:31:55:31:60 | locale |
| json-stringify.jsx:31:55:31:60 | locale |
| json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) |
| json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) |
| jwt-server.js:7:9:7:35 | taint |
| jwt-server.js:7:9:7:35 | taint |
| jwt-server.js:7:17:7:35 | req.param("wobble") |
@@ -1509,6 +1525,24 @@ edges
| jquery.js:28:5:28:26 | window. ... .search | jquery.js:28:5:28:43 | window. ... ?', '') |
| jquery.js:34:13:34:16 | hash | jquery.js:34:5:34:25 | '<b>' + ... '</b>' |
| jquery.js:34:13:34:16 | hash | jquery.js:34:5:34:25 | '<b>' + ... '</b>' |
| json-stringify.jsx:5:9:5:36 | locale | json-stringify.jsx:11:51:11:56 | locale |
| json-stringify.jsx:5:9:5:36 | locale | json-stringify.jsx:19:56:19:61 | locale |
| json-stringify.jsx:5:9:5:36 | locale | json-stringify.jsx:31:55:31:60 | locale |
| json-stringify.jsx:5:9:5:36 | locale | json-stringify.jsx:31:55:31:60 | locale |
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:5:9:5:36 | locale |
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:5:9:5:36 | locale |
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:5:9:5:36 | locale |
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:5:9:5:36 | locale |
| json-stringify.jsx:11:16:11:58 | `https: ... ocale}` | json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) |
| json-stringify.jsx:11:16:11:58 | `https: ... ocale}` | json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) |
| json-stringify.jsx:11:51:11:56 | locale | json-stringify.jsx:11:16:11:58 | `https: ... ocale}` |
| json-stringify.jsx:19:16:19:63 | `https: ... ocale}` | json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) |
| json-stringify.jsx:19:16:19:63 | `https: ... ocale}` | json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) |
| json-stringify.jsx:19:56:19:61 | locale | json-stringify.jsx:19:16:19:63 | `https: ... ocale}` |
| json-stringify.jsx:31:55:31:60 | locale | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
| json-stringify.jsx:31:55:31:60 | locale | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
| json-stringify.jsx:31:55:31:60 | locale | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
| json-stringify.jsx:31:55:31:60 | locale | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
| jwt-server.js:7:9:7:35 | taint | jwt-server.js:9:16:9:20 | taint |
| jwt-server.js:7:9:7:35 | taint | jwt-server.js:9:16:9:20 | taint |
| jwt-server.js:7:17:7:35 | req.param("wobble") | jwt-server.js:7:9:7:35 | taint |
@@ -2241,6 +2275,8 @@ edges
| jquery.js:27:5:27:25 | hash.re ... #', '') | jquery.js:18:14:18:33 | window.location.hash | jquery.js:27:5:27:25 | hash.re ... #', '') | Cross-site scripting vulnerability due to $@. | jquery.js:18:14:18:33 | window.location.hash | user-provided value |
| jquery.js:28:5:28:43 | window. ... ?', '') | jquery.js:28:5:28:26 | window. ... .search | jquery.js:28:5:28:43 | window. ... ?', '') | Cross-site scripting vulnerability due to $@. | jquery.js:28:5:28:26 | window. ... .search | user-provided value |
| jquery.js:34:5:34:25 | '<b>' + ... '</b>' | jquery.js:18:14:18:33 | window.location.hash | jquery.js:34:5:34:25 | '<b>' + ... '</b>' | Cross-site scripting vulnerability due to $@. | jquery.js:18:14:18:33 | window.location.hash | user-provided value |
| json-stringify.jsx:31:40:31:61 | JSON.st ... locale) | json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) | Cross-site scripting vulnerability due to $@. | json-stringify.jsx:5:18:5:36 | req.param("locale") | user-provided value |
| json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) | json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) | Cross-site scripting vulnerability due to $@. | json-stringify.jsx:5:18:5:36 | req.param("locale") | user-provided value |
| jwt-server.js:11:19:11:29 | decoded.foo | jwt-server.js:7:17:7:35 | req.param("wobble") | jwt-server.js:11:19:11:29 | decoded.foo | Cross-site scripting vulnerability due to $@. | jwt-server.js:7:17:7:35 | req.param("wobble") | user-provided value |
| nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | nodemailer.js:13:50:13:66 | req.query.message | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | HTML injection vulnerability due to $@. | nodemailer.js:13:50:13:66 | req.query.message | user-provided value |
| optionalSanitizer.js:6:18:6:23 | target | optionalSanitizer.js:2:16:2:39 | documen ... .search | optionalSanitizer.js:6:18:6:23 | target | Cross-site scripting vulnerability due to $@. | optionalSanitizer.js:2:16:2:39 | documen ... .search | user-provided value |

34
javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected
View File

@@ -431,6 +431,22 @@ nodes
| jquery.js:34:5:34:25 | '<b>' + ... '</b>' |
| jquery.js:34:5:34:25 | '<b>' + ... '</b>' |
| jquery.js:34:13:34:16 | hash |
| json-stringify.jsx:5:9:5:36 | locale |
| json-stringify.jsx:5:9:5:36 | locale |
| json-stringify.jsx:5:18:5:36 | req.param("locale") |
| json-stringify.jsx:5:18:5:36 | req.param("locale") |
| json-stringify.jsx:5:18:5:36 | req.param("locale") |
| json-stringify.jsx:11:16:11:58 | `https: ... ocale}` |
| json-stringify.jsx:11:51:11:56 | locale |
| json-stringify.jsx:19:16:19:63 | `https: ... ocale}` |
| json-stringify.jsx:19:56:19:61 | locale |
| json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
| json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
| json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
| json-stringify.jsx:31:55:31:60 | locale |
| json-stringify.jsx:31:55:31:60 | locale |
| json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) |
| json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) |
| jwt-server.js:7:9:7:35 | taint |
| jwt-server.js:7:9:7:35 | taint |
| jwt-server.js:7:17:7:35 | req.param("wobble") |
@@ -1559,6 +1575,24 @@ edges
| jquery.js:28:5:28:26 | window. ... .search | jquery.js:28:5:28:43 | window. ... ?', '') |
| jquery.js:34:13:34:16 | hash | jquery.js:34:5:34:25 | '<b>' + ... '</b>' |
| jquery.js:34:13:34:16 | hash | jquery.js:34:5:34:25 | '<b>' + ... '</b>' |
| json-stringify.jsx:5:9:5:36 | locale | json-stringify.jsx:11:51:11:56 | locale |
| json-stringify.jsx:5:9:5:36 | locale | json-stringify.jsx:19:56:19:61 | locale |
| json-stringify.jsx:5:9:5:36 | locale | json-stringify.jsx:31:55:31:60 | locale |
| json-stringify.jsx:5:9:5:36 | locale | json-stringify.jsx:31:55:31:60 | locale |
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:5:9:5:36 | locale |
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:5:9:5:36 | locale |
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:5:9:5:36 | locale |
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:5:9:5:36 | locale |
| json-stringify.jsx:11:16:11:58 | `https: ... ocale}` | json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) |
| json-stringify.jsx:11:16:11:58 | `https: ... ocale}` | json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) |
| json-stringify.jsx:11:51:11:56 | locale | json-stringify.jsx:11:16:11:58 | `https: ... ocale}` |
| json-stringify.jsx:19:16:19:63 | `https: ... ocale}` | json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) |
| json-stringify.jsx:19:16:19:63 | `https: ... ocale}` | json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) |
| json-stringify.jsx:19:56:19:61 | locale | json-stringify.jsx:19:16:19:63 | `https: ... ocale}` |
| json-stringify.jsx:31:55:31:60 | locale | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
| json-stringify.jsx:31:55:31:60 | locale | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
| json-stringify.jsx:31:55:31:60 | locale | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
| json-stringify.jsx:31:55:31:60 | locale | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
| jwt-server.js:7:9:7:35 | taint | jwt-server.js:9:16:9:20 | taint |
| jwt-server.js:7:9:7:35 | taint | jwt-server.js:9:16:9:20 | taint |
| jwt-server.js:7:17:7:35 | req.param("wobble") | jwt-server.js:7:9:7:35 | taint |

42
javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/json-stringify.jsx Normal file
View File

@@ -0,0 +1,42 @@
var express = require("express");
var app = express();
app.get("/some/path", function (req, res) {
const locale = req.param("locale");
const breadcrumbList = [
{
"@type": "ListItem",
position: 1,
item: {
"@id": `https://example.com/some?locale=${locale}`,
name: "Some",
},
},
{
"@type": "ListItem",
position: 2,
item: {
"@id": `https://example.com/some/path?locale=${locale}`,
name: "Path",
},
},
];
const jsonLD = {
"@context": "https://schema.org",
"@type": "BreadcrumbList",
itemListElement: breadcrumbList,
};
<script
type="application/ld+json"
dangerouslySetInnerHTML={{ __html: JSON.stringify(locale) }} // NOT OK
/>;
<script
type="application/ld+json"
dangerouslySetInnerHTML={{ __html: JSON.stringify(jsonLD) }} // NOT OK
/>;
<script
type="application/ld+json"
dangerouslySetInnerHTML={{ __html: JSON.stringify({}) }} // OK
/>;
<script type="application/ld+json">{ JSON.stringify(jsonLD) }</script> // OK
});