Skip to content

Commit a75a004

Browse files
am0o0hmac
authored andcommitted
add more additional steps, change parse* sinks to reciever of them
1 parent 474a4f8 commit a75a004

File tree

2 files changed

+29
-20
lines changed

2 files changed

+29
-20
lines changed

ruby/ql/lib/codeql/ruby/frameworks/Yaml.qll

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,36 +27,45 @@ private class YamlParseStep extends AdditionalTaintStep {
2727
exists(API::Node yamlParserMethod |
2828
succ = yamlParserMethod.getReturn().asSource() and
2929
(
30-
yamlParserMethod = yamlNode().getMethod(["parse", "parse_stream"]) and
30+
yamlParserMethod = yamlLibrary().getMethod(["parse", "parse_stream"]) and
3131
pred =
3232
[yamlParserMethod.getParameter(0), yamlParserMethod.getKeywordParameter("yaml")].asSink()
3333
or
34-
yamlParserMethod = yamlNode().getMethod("parse_file") and
34+
yamlParserMethod = yamlLibrary().getMethod("parse_file") and
3535
pred =
3636
[yamlParserMethod.getParameter(0), yamlParserMethod.getKeywordParameter("filename")]
3737
.asSink()
3838
)
3939
)
4040
or
41-
exists(API::Node parseSuccessors | parseSuccessors = yamlParseChildNodeAccess(_) |
41+
exists(API::Node parseSuccessors | parseSuccessors = yamlNode() |
4242
succ =
4343
[
4444
parseSuccessors.getMethod("to_ruby").getReturn().asSource(),
4545
parseSuccessors.getMethod("to_ruby").getReturn().getAnElement().asSource()
4646
] and
4747
pred = parseSuccessors.asSource()
4848
)
49+
or
50+
exists(API::Node parseSuccessors | parseSuccessors = yamlNode() |
51+
succ =
52+
[
53+
parseSuccessors.getMethod(_).getBlock().getParameter(_).asSource(),
54+
parseSuccessors.getMethod(_).getReturn().asSource()
55+
] and
56+
pred = parseSuccessors.asSource()
57+
)
4958
}
5059
}
5160

52-
API::Node yamlParseChildNodeAccess(API::Node source) {
53-
source = yamlNode().getMethod(["parse", "parse_stream"]).getReturn() and source = result
61+
API::Node yamlNode() {
62+
result = yamlLibrary().getMethod(["parse", "parse_stream", "parse_file"]).getReturn()
5463
or
55-
result = yamlParseChildNodeAccess(source).getMethod(_).getReturn()
64+
result = yamlNode().getMethod(_).getReturn()
5665
or
57-
result = yamlParseChildNodeAccess(source).getMethod(_).getBlock().getParameter(_)
66+
result = yamlNode().getMethod(_).getBlock().getParameter(_)
5867
or
59-
result = yamlParseChildNodeAccess(source).getAnElement()
68+
result = yamlNode().getAnElement()
6069
}
6170

62-
private API::Node yamlNode() { result = API::getTopLevelMember(["YAML", "Psych"]) }
71+
API::Node yamlLibrary() { result = API::getTopLevelMember(["YAML", "Psych"]) }

ruby/ql/lib/codeql/ruby/security/UnsafeDeserializationCustomizations.qll

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -83,30 +83,30 @@ module UnsafeDeserialization {
8383
class YamlLoadArgument extends Sink {
8484
YamlLoadArgument() {
8585
// Note: this is safe in psych/yaml >= 4.0.0.
86-
this = yamlNode().getAMethodCall("load").getArgument(0)
86+
this = yamlLibrary().getAMethodCall("load").getArgument(0)
8787
or
8888
this =
89-
yamlNode().getAMethodCall(["unsafe_load_file", "unsafe_load", "load_stream"]).getArgument(0)
89+
yamlLibrary()
90+
.getAMethodCall(["unsafe_load_file", "unsafe_load", "load_stream"])
91+
.getArgument(0)
9092
or
91-
this = yamlNode().getAMethodCall(["unsafe_load", "load_stream"]).getKeywordArgument("yaml")
93+
this = yamlLibrary().getAMethodCall(["unsafe_load", "load_stream"]).getKeywordArgument("yaml")
9294
or
93-
this = yamlNode().getAMethodCall("unsafe_load_file").getKeywordArgument("filename")
95+
this = yamlLibrary().getAMethodCall("unsafe_load_file").getKeywordArgument("filename")
9496
}
9597
}
9698

97-
private API::Node yamlNode() { result = API::getTopLevelMember(["YAML", "Psych"]) }
98-
9999
/**
100100
* An argument in a call to `YAML.parse*`, considered a sink for unsafe deserialization
101101
* if there is a call to `to_ruby` on the returned value of any Successor.
102102
*/
103103
class YamlParseArgument extends Sink {
104104
YamlParseArgument() {
105-
this =
106-
[
107-
yamlParseChildNodeAccess(_).getMethod("to_ruby").getReturn().asSource(),
108-
yamlParseChildNodeAccess(_).getMethod("to_ruby").getReturn().getAnElement().asSource()
109-
]
105+
exists(API::Node toRubyReceiver |
106+
toRubyReceiver = yamlNode() and this = toRubyReceiver.asSource()
107+
|
108+
exists(toRubyReceiver.getMethod("to_ruby"))
109+
)
110110
}
111111
}
112112

0 commit comments

Comments
 (0)