Skip to content

Commit 8bbbb95

Browse files
authored
Make ZipSlip module classes private and push Sanitizer definition to ZipSlipCustomization.qll
1 parent d8eafea commit 8bbbb95

File tree

1 file changed

+13
-7
lines changed

1 file changed

+13
-7
lines changed

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

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ module ZipSlip {
3838
/**
3939
* A call to `Zlib::GzipReader.open(path)`, considered a flow source.
4040
*/
41-
class GzipReaderOpen extends Source {
41+
private class GzipReaderOpen extends Source {
4242
GzipReaderOpen() {
4343
this = API::getTopLevelMember("Zlib").getMember("GzipReader").getReturn("open").asSource() and
4444
// If argument refers to a string object, then it's a hardcoded path and
@@ -54,7 +54,7 @@ module ZipSlip {
5454
/**
5555
* A call to `Gem::Package::TarReader.new(file_stream)`, considered a flow source.
5656
*/
57-
class TarReaderInstance extends Source {
57+
private class TarReaderInstance extends Source {
5858
TarReaderInstance() {
5959
this =
6060
API::getTopLevelMember("Gem")
@@ -75,7 +75,7 @@ module ZipSlip {
7575
/**
7676
* A call to `Zip::File.open(path)`, considered a flow source.
7777
*/
78-
class ZipFileOpen extends Source {
78+
private class ZipFileOpen extends Source {
7979
ZipFileOpen() {
8080
this = API::getTopLevelMember("Zip").getMember("File").getReturn("open").asSource() and
8181
// If argument refers to a string object, then it's a hardcoded path and
@@ -91,25 +91,31 @@ module ZipSlip {
9191
/**
9292
* A comparison with a constant string, considered as a sanitizer-guard.
9393
*/
94-
class StringConstCompareAsSanitizer extends Sanitizer, StringConstCompareBarrier { }
94+
private class StringConstCompareAsSanitizer extends Sanitizer, StringConstCompareBarrier { }
9595

9696
/**
9797
* An inclusion check against an array of constant strings, considered as a
9898
* sanitizer-guard.
9999
*/
100-
class StringConstArrayInclusionCallAsSanitizer extends Sanitizer,
100+
private class StringConstArrayInclusionCallAsSanitizer extends Sanitizer,
101101
StringConstArrayInclusionCallBarrier { }
102102

103103
/**
104104
* A sanitizer like `File.expand_path(path).start_with?` where `path` is a path of a single entry inside the archive.
105-
* It is assumed that if `File.expand_path` is called, it is to verify the path is safe so there's no modelling of `start_with?` or other comparisons to avoid false-negatives.
105+
* It is assumed that if `File.expand_path` is called, it is to verify the path is safe so there's no modeling of `start_with?` or other comparisons to avoid false-negatives.
106106
*/
107-
class ExpandedPathStartsWithAsSanitizer extends Sanitizer {
107+
private class ExpandedPathStartsWithAsSanitizer extends Sanitizer {
108108
ExpandedPathStartsWithAsSanitizer() {
109109
exists(DataFlow::CallNode cn |
110110
cn.getMethodName() = "expand_path" and
111111
this = cn.getArgument(0)
112112
)
113113
}
114114
}
115+
116+
117+
/**
118+
* Existing PathSanitization model created for regular path traversals
119+
*/
120+
private class PathSanitizationAsSanitizer extends Sanitizer instanceof Path::PathSanitization { }
115121
}

0 commit comments

Comments
 (0)