Merge pull request #12887 from asgerf/js/unsafe-yaml-deserialization

JS: Update model of js-yaml
This commit is contained in:
Asger F
2023-05-01 09:57:20 +02:00
committed by GitHub
7 changed files with 89 additions and 36 deletions

View File

@@ -491,6 +491,7 @@ module API {
* In other words, the value of a use of `that` may flow into the right-hand side of a
* definition of this node.
*/
pragma[inline]
predicate refersTo(Node that) { this.asSink() = that.getAValueReachableFromSource() }
/**

View File

@@ -25,25 +25,36 @@ module UnsafeDeserialization {
/** A source of remote user input, considered as a flow source for unsafe deserialization. */
class RemoteFlowSourceAsSource extends Source instanceof RemoteFlowSource { }
private API::Node unsafeYamlSchema() {
result = API::moduleImport("js-yaml").getMember("DEFAULT_FULL_SCHEMA") // from older versions
or
result = API::moduleImport("js-yaml-js-types").getMember(["all", "function"])
or
result = unsafeYamlSchema().getMember("extend").getReturn()
or
exists(API::CallNode call |
call.getAParameter().refersTo(unsafeYamlSchema()) and
call.getCalleeName() = "extend" and
result = call.getReturn()
)
}
/**
* An expression passed to one of the unsafe load functions of the `js-yaml` package.
*
* `js-yaml` since v4 defaults to being safe, but is unsafe when invoked with a schema
* that permits unsafe values.
*/
class JsYamlUnsafeLoad extends Sink {
JsYamlUnsafeLoad() {
exists(DataFlow::ModuleImportNode mi | mi.getPath() = "js-yaml" |
// the first argument to a call to `load` or `loadAll`
exists(string n | n = "load" or n = "loadAll" | this = mi.getAMemberCall(n).getArgument(0))
or
// the first argument to a call to `safeLoad` or `safeLoadAll` where
// the schema is specified to be `DEFAULT_FULL_SCHEMA`
exists(string n, DataFlow::CallNode c, DataFlow::Node fullSchema |
n = "safeLoad" or n = "safeLoadAll"
|
c = mi.getAMemberCall(n) and
this = c.getArgument(0) and
fullSchema = c.getOptionArgument(c.getNumArgument() - 1, "schema") and
mi.getAPropertyRead("DEFAULT_FULL_SCHEMA").flowsTo(fullSchema)
)
exists(API::CallNode call |
// Note: we include the old 'safeLoad' and 'safeLoadAll' functon because they were also unsafe when invoked with an unsafe schema.
call =
API::moduleImport("js-yaml")
.getMember(["load", "loadAll", "safeLoad", "safeLoadAll"])
.getACall() and
call.getAParameter().getMember("schema").refersTo(unsafeYamlSchema()) and
this = call.getArgument(0)
)
}
}

View File

@@ -0,0 +1,5 @@
---
category: minorAnalysis
---
* The `js/unsafe-deserialization` query no longer flags deserialization through the `js-yaml` library, except
when it is used with an unsafe schema.

View File

@@ -0,0 +1,3 @@
import javascript
import semmle.javascript.security.dataflow.UnsafeDeserializationQuery
import testUtilities.ConsistencyChecking

View File

@@ -1,23 +1,43 @@
nodes
| tst.js:7:22:7:36 | req.params.data |
| tst.js:7:22:7:36 | req.params.data |
| tst.js:7:22:7:36 | req.params.data |
| tst.js:8:25:8:39 | req.params.data |
| tst.js:8:25:8:39 | req.params.data |
| tst.js:8:25:8:39 | req.params.data |
| tst.js:12:26:12:40 | req.params.data |
| tst.js:12:26:12:40 | req.params.data |
| tst.js:12:26:12:40 | req.params.data |
| tst.js:13:29:13:43 | req.params.data |
| tst.js:13:29:13:43 | req.params.data |
| tst.js:13:29:13:43 | req.params.data |
| tst.js:13:22:13:36 | req.params.data |
| tst.js:13:22:13:36 | req.params.data |
| tst.js:13:22:13:36 | req.params.data |
| tst.js:14:25:14:39 | req.params.data |
| tst.js:14:25:14:39 | req.params.data |
| tst.js:14:25:14:39 | req.params.data |
| tst.js:15:26:15:40 | req.params.data |
| tst.js:15:26:15:40 | req.params.data |
| tst.js:15:26:15:40 | req.params.data |
| tst.js:16:29:16:43 | req.params.data |
| tst.js:16:29:16:43 | req.params.data |
| tst.js:16:29:16:43 | req.params.data |
| tst.js:20:22:20:36 | req.params.data |
| tst.js:20:22:20:36 | req.params.data |
| tst.js:20:22:20:36 | req.params.data |
| tst.js:21:22:21:36 | req.params.data |
| tst.js:21:22:21:36 | req.params.data |
| tst.js:21:22:21:36 | req.params.data |
| tst.js:24:22:24:36 | req.params.data |
| tst.js:24:22:24:36 | req.params.data |
| tst.js:24:22:24:36 | req.params.data |
| tst.js:25:22:25:36 | req.params.data |
| tst.js:25:22:25:36 | req.params.data |
| tst.js:25:22:25:36 | req.params.data |
edges
| tst.js:7:22:7:36 | req.params.data | tst.js:7:22:7:36 | req.params.data |
| tst.js:8:25:8:39 | req.params.data | tst.js:8:25:8:39 | req.params.data |
| tst.js:12:26:12:40 | req.params.data | tst.js:12:26:12:40 | req.params.data |
| tst.js:13:29:13:43 | req.params.data | tst.js:13:29:13:43 | req.params.data |
| tst.js:13:22:13:36 | req.params.data | tst.js:13:22:13:36 | req.params.data |
| tst.js:14:25:14:39 | req.params.data | tst.js:14:25:14:39 | req.params.data |
| tst.js:15:26:15:40 | req.params.data | tst.js:15:26:15:40 | req.params.data |
| tst.js:16:29:16:43 | req.params.data | tst.js:16:29:16:43 | req.params.data |
| tst.js:20:22:20:36 | req.params.data | tst.js:20:22:20:36 | req.params.data |
| tst.js:21:22:21:36 | req.params.data | tst.js:21:22:21:36 | req.params.data |
| tst.js:24:22:24:36 | req.params.data | tst.js:24:22:24:36 | req.params.data |
| tst.js:25:22:25:36 | req.params.data | tst.js:25:22:25:36 | req.params.data |
#select
| tst.js:7:22:7:36 | req.params.data | tst.js:7:22:7:36 | req.params.data | tst.js:7:22:7:36 | req.params.data | Unsafe deserialization depends on a $@. | tst.js:7:22:7:36 | req.params.data | user-provided value |
| tst.js:8:25:8:39 | req.params.data | tst.js:8:25:8:39 | req.params.data | tst.js:8:25:8:39 | req.params.data | Unsafe deserialization depends on a $@. | tst.js:8:25:8:39 | req.params.data | user-provided value |
| tst.js:12:26:12:40 | req.params.data | tst.js:12:26:12:40 | req.params.data | tst.js:12:26:12:40 | req.params.data | Unsafe deserialization depends on a $@. | tst.js:12:26:12:40 | req.params.data | user-provided value |
| tst.js:13:29:13:43 | req.params.data | tst.js:13:29:13:43 | req.params.data | tst.js:13:29:13:43 | req.params.data | Unsafe deserialization depends on a $@. | tst.js:13:29:13:43 | req.params.data | user-provided value |
| tst.js:13:22:13:36 | req.params.data | tst.js:13:22:13:36 | req.params.data | tst.js:13:22:13:36 | req.params.data | Unsafe deserialization depends on a $@. | tst.js:13:22:13:36 | req.params.data | user-provided value |
| tst.js:14:25:14:39 | req.params.data | tst.js:14:25:14:39 | req.params.data | tst.js:14:25:14:39 | req.params.data | Unsafe deserialization depends on a $@. | tst.js:14:25:14:39 | req.params.data | user-provided value |
| tst.js:15:26:15:40 | req.params.data | tst.js:15:26:15:40 | req.params.data | tst.js:15:26:15:40 | req.params.data | Unsafe deserialization depends on a $@. | tst.js:15:26:15:40 | req.params.data | user-provided value |
| tst.js:16:29:16:43 | req.params.data | tst.js:16:29:16:43 | req.params.data | tst.js:16:29:16:43 | req.params.data | Unsafe deserialization depends on a $@. | tst.js:16:29:16:43 | req.params.data | user-provided value |
| tst.js:20:22:20:36 | req.params.data | tst.js:20:22:20:36 | req.params.data | tst.js:20:22:20:36 | req.params.data | Unsafe deserialization depends on a $@. | tst.js:20:22:20:36 | req.params.data | user-provided value |
| tst.js:21:22:21:36 | req.params.data | tst.js:21:22:21:36 | req.params.data | tst.js:21:22:21:36 | req.params.data | Unsafe deserialization depends on a $@. | tst.js:21:22:21:36 | req.params.data | user-provided value |
| tst.js:24:22:24:36 | req.params.data | tst.js:24:22:24:36 | req.params.data | tst.js:24:22:24:36 | req.params.data | Unsafe deserialization depends on a $@. | tst.js:24:22:24:36 | req.params.data | user-provided value |
| tst.js:25:22:25:36 | req.params.data | tst.js:25:22:25:36 | req.params.data | tst.js:25:22:25:36 | req.params.data | Unsafe deserialization depends on a $@. | tst.js:25:22:25:36 | req.params.data | user-provided value |

View File

@@ -4,11 +4,24 @@ var express = require('express');
var app = express();
app.post('/store/:id', function(req, res) {
let data;
data = jsyaml.load(req.params.data); // NOT OK
data = jsyaml.loadAll(req.params.data); // NOT OK
data = jsyaml.load(req.params.data); // OK
data = jsyaml.loadAll(req.params.data); // OK
data = jsyaml.safeLoad(req.params.data); // OK
data = jsyaml.safeLoadAll(req.params.data); // OK
let unsafeConfig = { schema: jsyaml.DEFAULT_FULL_SCHEMA };
data = jsyaml.load(req.params.data, unsafeConfig); // NOT OK
data = jsyaml.loadAll(req.params.data, unsafeConfig); // NOT OK
data = jsyaml.safeLoad(req.params.data, unsafeConfig); // NOT OK
data = jsyaml.safeLoadAll(req.params.data, unsafeConfig); // NOT OK
data = jsyaml.load(req.params.data, { schema: jsyaml.DEFAULT_SCHEMA }); // OK
data = jsyaml.load(req.params.data, { schema: jsyaml.DEFAULT_SCHEMA.extend(require('js-yaml-js-types').all) }); // NOT OK
data = jsyaml.load(req.params.data, { schema: jsyaml.DEFAULT_SCHEMA.extend(require('js-yaml-js-types').function) }); // NOT OK
data = jsyaml.load(req.params.data, { schema: jsyaml.DEFAULT_SCHEMA.extend(require('js-yaml-js-types').undefined) }); // OK
data = jsyaml.load(req.params.data, { schema: require('js-yaml-js-types').all.extend(jsyaml.DEFAULT_SCHEMA) }); // NOT OK
data = jsyaml.load(req.params.data, { schema: require('js-yaml-js-types').function.extend(jsyaml.DEFAULT_SCHEMA) }); // NOT OK
data = jsyaml.load(req.params.data, { schema: require('js-yaml-js-types').undefined.extend(jsyaml.DEFAULT_SCHEMA) }); // OK
});