Merge pull request #4014 from erik-krogh/stringify

Approved by esbena
This commit is contained in:
CodeQL CI
2020-08-10 07:50:21 +01:00
committed by GitHub
11 changed files with 105 additions and 25 deletions

View File

@@ -67,7 +67,7 @@ predicate isBackslashEscape(StringReplaceCall mce, DataFlow::RegExpLiteralNode r
*/
predicate allBackslashesEscaped(DataFlow::Node nd) {
// `JSON.stringify` escapes backslashes
nd = DataFlow::globalVarRef("JSON").getAMemberCall("stringify")
nd instanceof JsonStringifyCall
or
// check whether `nd` itself escapes backslashes
exists(DataFlow::RegExpLiteralNode rel | isBackslashEscape(nd, rel) |

View File

@@ -34,6 +34,7 @@ import semmle.javascript.InclusionTests
import semmle.javascript.JSDoc
import semmle.javascript.JSON
import semmle.javascript.JsonParsers
import semmle.javascript.JsonStringifiers
import semmle.javascript.JSX
import semmle.javascript.Lines
import semmle.javascript.Locations

View File

@@ -0,0 +1,34 @@
/**
* Provides classes for working with JSON serializers.
*/
import javascript
/**
* A call to a JSON stringifier such as `JSON.stringify` or `require("util").inspect`.
*/
class JsonStringifyCall extends DataFlow::CallNode {
JsonStringifyCall() {
exists(DataFlow::SourceNode callee | this = callee.getACall() |
callee = DataFlow::globalVarRef("JSON").getAPropertyRead("stringify") or
callee = DataFlow::moduleMember("json3", "stringify") or
callee =
DataFlow::moduleImport(["json-stringify-safe", "json-stable-stringify", "stringify-object",
"fast-json-stable-stringify", "fast-safe-stringify", "javascript-stringify",
"js-stringify"]) or
// require("util").inspect() and similar
callee = DataFlow::moduleMember("util", "inspect") or
callee = DataFlow::moduleImport(["pretty-format", "object-inspect"])
)
}
/**
* Gets the data flow node holding the input object to be stringified.
*/
DataFlow::Node getInput() { result = getArgument(0) }
/**
* Gets the data flow node holding the resulting string.
*/
DataFlow::SourceNode getOutput() { result = this }
}

View File

@@ -543,8 +543,8 @@ module TaintTracking {
/**
* A taint propagating data flow edge arising from JSON unparsing.
*/
private class JsonStringifyTaintStep extends AdditionalTaintStep, DataFlow::MethodCallNode {
JsonStringifyTaintStep() { this = DataFlow::globalVarRef("JSON").getAMemberCall("stringify") }
private class JsonStringifyTaintStep extends AdditionalTaintStep, DataFlow::CallNode {
JsonStringifyTaintStep() { this instanceof JsonStringifyCall }
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
pred = getArgument(0) and succ = this
@@ -693,18 +693,6 @@ module TaintTracking {
}
}
/**
* A taint step through the Node.JS function `util.inspect(..)`.
*/
class UtilInspectTaintStep extends AdditionalTaintStep, DataFlow::InvokeNode {
UtilInspectTaintStep() { this = DataFlow::moduleImport("util").getAMemberCall("inspect") }
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
succ = this and
this.getAnArgument() = pred
}
}
private module RegExpCaptureSteps {
/** Gets a reference to a string derived from the most recent RegExp match, such as `RegExp.$1`. */
private DataFlow::PropRead getAStaticCaptureRef() {

View File

@@ -28,9 +28,7 @@ private class RemoteFlowPassword extends HeuristicSource, RemoteFlowSource {
*/
private class JSONStringifyAsCommandInjectionSource extends HeuristicSource,
CommandInjection::Source {
JSONStringifyAsCommandInjectionSource() {
this = DataFlow::globalVarRef("JSON").getAMemberCall("stringify")
}
JSONStringifyAsCommandInjectionSource() { this instanceof JsonStringifyCall }
override string getSourceType() { result = "a string from JSON.stringify" }
}

View File

@@ -202,10 +202,9 @@ module CleartextLogging {
exists(DataFlow::PropWrite write, DataFlow::PropRead read |
read = write.getRhs()
or
exists(DataFlow::MethodCallNode stringify |
stringify = write.getRhs() and
stringify = DataFlow::globalVarRef("JSON").getAMethodCall("stringify") and
stringify.getArgument(0) = read
exists(JsonStringifyCall stringify |
stringify.getOutput() = write.getRhs() and
stringify.getInput() = read
)
|
not exists(write.getPropertyName()) and

View File

@@ -36,7 +36,7 @@ module ImproperCodeSanitization {
* A call to `JSON.stringify()` seen as a source for improper code sanitization
*/
class JSONStringifyAsSource extends Source {
JSONStringifyAsSource() { this = DataFlow::globalVarRef("JSON").getAMemberCall("stringify") }
JSONStringifyAsSource() { this instanceof JsonStringifyCall }
}
/**

View File

@@ -49,8 +49,7 @@ module PostMessageStar {
exists(DataFlow::InvokeNode toString | toString = trg |
toString.(DataFlow::MethodCallNode).calls(src, "toString")
or
toString = DataFlow::globalVarRef("JSON").getAMemberCall("stringify") and
src = toString.getArgument(0)
src = toString.(JsonStringifyCall).getInput()
) and
inlbl instanceof PartiallyTaintedObject and
outlbl.isTaint()

View File

@@ -68,6 +68,18 @@ typeInferenceMismatch
| exceptions.js:158:13:158:20 | source() | exceptions.js:161:10:161:10 | e |
| importedReactComponent.jsx:4:40:4:47 | source() | exportedReactComponent.jsx:2:10:2:19 | props.text |
| indexOf.js:4:11:4:18 | source() | indexOf.js:9:10:9:10 | x |
| json-stringify.js:2:16:2:23 | source() | json-stringify.js:5:8:5:29 | JSON.st ... source) |
| json-stringify.js:2:16:2:23 | source() | json-stringify.js:9:8:9:47 | require ... source) |
| json-stringify.js:2:16:2:23 | source() | json-stringify.js:10:8:10:42 | require ... source) |
| json-stringify.js:2:16:2:23 | source() | json-stringify.js:11:8:11:41 | require ... source) |
| json-stringify.js:2:16:2:23 | source() | json-stringify.js:12:8:12:52 | require ... source) |
| json-stringify.js:2:16:2:23 | source() | json-stringify.js:13:8:13:45 | require ... source) |
| json-stringify.js:2:16:2:23 | source() | json-stringify.js:14:8:14:46 | require ... source) |
| json-stringify.js:2:16:2:23 | source() | json-stringify.js:15:8:15:38 | require ... source) |
| json-stringify.js:2:16:2:23 | source() | json-stringify.js:16:8:16:38 | require ... source) |
| json-stringify.js:2:16:2:23 | source() | json-stringify.js:17:8:17:39 | require ... source) |
| json-stringify.js:2:16:2:23 | source() | json-stringify.js:18:8:18:40 | require ... source) |
| json-stringify.js:3:15:3:22 | source() | json-stringify.js:8:8:8:31 | jsonStr ... (taint) |
| nested-props.js:4:13:4:20 | source() | nested-props.js:5:10:5:14 | obj.x |
| nested-props.js:9:18:9:25 | source() | nested-props.js:10:10:10:16 | obj.x.y |
| nested-props.js:35:13:35:20 | source() | nested-props.js:36:10:36:20 | doLoad(obj) |

View File

@@ -0,0 +1,19 @@
function foo() {
let source = source();
let taint = source();
sink(JSON.stringify(source)); // NOT OK
var jsonStringifySafe = require("json-stringify-safe");
sink(jsonStringifySafe(taint)); // NOT OK
sink(require("json-stable-stringify")(source)); // NOT OK
sink(require("stringify-object")(source)); // NOT OK
sink(require("json3").stringify(source)); // NOT OK
sink(require("fast-json-stable-stringify")(source)); // NOT OK
sink(require("fast-safe-stringify")(source)); // NOT OK
sink(require("javascript-stringify")(source)); // NOT OK
sink(require("js-stringify")(source)); // NOT OK
sink(require("util").inspect(source)); // NOT OK
sink(require("pretty-format")(source)); // NOT OK
sink(require("object-inspect")(source)); // NOT OK
}