Skip to content

Commit 1acc0d2

Browse files
committed
JS: Update model of js-yaml
1 parent b6a7661 commit 1acc0d2

File tree

2 files changed

+26
-14
lines changed

2 files changed

+26
-14
lines changed

javascript/ql/lib/semmle/javascript/ApiGraphs.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,7 @@ module API {
491491
* In other words, the value of a use of `that` may flow into the right-hand side of a
492492
* definition of this node.
493493
*/
494+
pragma[inline]
494495
predicate refersTo(Node that) { this.asSink() = that.getAValueReachableFromSource() }
495496

496497
/**

javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDeserializationCustomizations.qll

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,25 +25,36 @@ module UnsafeDeserialization {
2525
/** A source of remote user input, considered as a flow source for unsafe deserialization. */
2626
class RemoteFlowSourceAsSource extends Source instanceof RemoteFlowSource { }
2727

28+
private API::Node unsafeYamlSchema() {
29+
result = API::moduleImport("js-yaml").getMember("DEFAULT_FULL_SCHEMA") // from older versions
30+
or
31+
result = API::moduleImport("js-yaml-js-types").getMember(["all", "function"])
32+
or
33+
result = unsafeYamlSchema().getMember("extend").getReceiver()
34+
or
35+
exists(API::CallNode call |
36+
call.getAParameter().refersTo(unsafeYamlSchema()) and
37+
call.getCalleeName() = "extend" and
38+
result = call.getReturn()
39+
)
40+
}
41+
2842
/**
2943
* An expression passed to one of the unsafe load functions of the `js-yaml` package.
44+
*
45+
* `js-yaml` since v4 defaults to being safe, but is unsafe when invoked with a schema
46+
* that permits unsafe values.
3047
*/
3148
class JsYamlUnsafeLoad extends Sink {
3249
JsYamlUnsafeLoad() {
33-
exists(DataFlow::ModuleImportNode mi | mi.getPath() = "js-yaml" |
34-
// the first argument to a call to `load` or `loadAll`
35-
exists(string n | n = "load" or n = "loadAll" | this = mi.getAMemberCall(n).getArgument(0))
36-
or
37-
// the first argument to a call to `safeLoad` or `safeLoadAll` where
38-
// the schema is specified to be `DEFAULT_FULL_SCHEMA`
39-
exists(string n, DataFlow::CallNode c, DataFlow::Node fullSchema |
40-
n = "safeLoad" or n = "safeLoadAll"
41-
|
42-
c = mi.getAMemberCall(n) and
43-
this = c.getArgument(0) and
44-
fullSchema = c.getOptionArgument(c.getNumArgument() - 1, "schema") and
45-
mi.getAPropertyRead("DEFAULT_FULL_SCHEMA").flowsTo(fullSchema)
46-
)
50+
exists(API::CallNode call |
51+
// Note: we include the old 'safeLoad' and 'safeLoadAll' functon because they were also unsafe when invoked with an unsafe schema.
52+
call =
53+
API::moduleImport("js-yaml")
54+
.getMember(["load", "loadAll", "safeLoad", "safeLoadAll"])
55+
.getACall() and
56+
call.getAParameter().getMember("schema").refersTo(unsafeYamlSchema()) and
57+
this = call.getArgument(0)
4758
)
4859
}
4960
}

0 commit comments

Comments
 (0)