Skip to content

Commit 7c4604e

Browse files
authoredApr 14, 2024··
Fix: Unix OS's should properly ignore the windows zip slipped files (#179)
* Fix: Unix OS's should properly ignore the windows zip slipped files as well. * Tests: resurrected the original zip-slip test as is as requested
1 parent c743527 commit 7c4604e

File tree

3 files changed

+66
-1
lines changed

3 files changed

+66
-1
lines changed
 

‎lib/extract.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ function Extract (opts) {
2121
// to avoid zip slip (writing outside of the destination), we resolve
2222
// the target path, and make sure it's nested in the intended
2323
// destination, or not extract it otherwise.
24-
var extractPath = path.join(opts.path, entry.path);
24+
// NOTE: Need to normalize to forward slashes for UNIX OS's to properly
25+
// ignore the zip slipped file entirely
26+
var extractPath = path.join(opts.path, entry.path.replace(/\\/g, '/'));
2527
if (extractPath.indexOf(opts.path) != 0) {
2628
return cb();
2729
}

‎test/uncompressed.js

+63
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,66 @@ test("do not extract zip slip archive", function (t) {
8484

8585
});
8686
});
87+
88+
function testZipSlipArchive(t, slipFileName, attackPathFactory){
89+
var archive = path.join(__dirname, '../testData/zip-slip', slipFileName);
90+
91+
temp.mkdir('node-zipslip-' + slipFileName, function (err, dirPath) {
92+
if (err) {
93+
throw err;
94+
}
95+
var attackPath = attackPathFactory(dirPath);
96+
CheckForSlip(attackPath, function(slipAlreadyExists){
97+
if(slipAlreadyExists){
98+
t.fail('Cannot check for slip because the slipped file already exists at "' + attackPath+ '"');
99+
t.end();
100+
}
101+
else{
102+
var unzipExtractor = unzip.Extract({ path: dirPath });
103+
unzipExtractor.on('error', function(err) {
104+
throw err;
105+
});
106+
unzipExtractor.on('close', testNoSlip);
107+
108+
fs.createReadStream(archive).pipe(unzipExtractor);
109+
}
110+
})
111+
112+
function CheckForSlip(path, resultCallback) {
113+
var fsCallback = function(err){ return resultCallback(!err); };
114+
if (fs.hasOwnProperty('access')) {
115+
var mode = fs.F_OK | (fs.constants && fs.constants.F_OK);
116+
return fs.access(path, mode, fsCallback);
117+
}
118+
// node 0.10
119+
return fs.stat(path, fsCallback);
120+
}
121+
122+
function testNoSlip() {
123+
CheckForSlip(attackPath, function(slipExists) {
124+
if (slipExists) {
125+
t.fail('evil file created from ' + slipFileName + ' at "' + attackPath + '"');
126+
fs.unlinkSync(attackPath);
127+
} else {
128+
t.pass('no zip slip from ' + slipFileName);
129+
}
130+
return t.end();
131+
})
132+
}
133+
});
134+
}
135+
136+
test("do not extract zip slip archive(Windows)", function (t) {
137+
var pathFactory;
138+
if(process.platform === "win32") {
139+
pathFactory = function(dirPath) { return '\\Temp\\evil.txt'; }
140+
}
141+
else{
142+
// UNIX should treat the backslashes as escapes not directory delimiters
143+
// will be a file with slashes in the name. Looks real weird.
144+
pathFactory = function(dirPath) { return path.join(dirPath, '..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\Temp\\evil.txt'); }
145+
}
146+
147+
testZipSlipArchive(t, 'zip-slip-win.zip', pathFactory);
148+
});
149+

‎testData/zip-slip/zip-slip-win.zip

547 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)
Please sign in to comment.