Skip to content

Commit

Permalink
Fixed path traversal issue GHSL-2020-198
Browse files Browse the repository at this point in the history
  • Loading branch information
@iacobnasca
iacobnasca committed Jan 27, 2021
1 parent 1d22ff6 commit 119dcad
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 27 deletions.
18 changes: 10 additions & 8 deletions adm-zip.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ var ZipEntry = require("./zipEntry"),

var isWin = /^win/.test(process.platform);

function canonical(p) {
var safeSuffix = pth.normalize(p).replace(/^(\.\.(\/|\\|$))+/, '');
return pth.join('./', safeSuffix);
}

module.exports = function (/**String*/input) {
var _zip = undefined,
Expand Down Expand Up @@ -469,12 +473,9 @@ module.exports = function (/**String*/input) {
throw new Error(Utils.Errors.NO_ENTRY);
}

var entryName = item.entryName;
var entryName = canonical(item.entryName);

var target = sanitize(targetPath,
outFileName && !item.isDirectory ? outFileName :
(maintainEntryPath ? entryName : pth.basename(entryName))
);
var target = sanitize(targetPath,outFileName && !item.isDirectory ? outFileName : (maintainEntryPath ? entryName : pth.basename(entryName)));

if (item.isDirectory) {
target = pth.resolve(target, "..");
Expand All @@ -485,7 +486,8 @@ module.exports = function (/**String*/input) {
if (!content) {
throw new Error(Utils.Errors.CANT_EXTRACT_FILE);
}
var childName = sanitize(targetPath, maintainEntryPath ? child.entryName : pth.basename(child.entryName));
var name = canonical(child.entryName)
var childName = sanitize(targetPath, maintainEntryPath ? name : pth.basename(name));

Utils.writeFileTo(childName, content, overwrite);
});
Expand Down Expand Up @@ -541,7 +543,7 @@ module.exports = function (/**String*/input) {
throw new Error(Utils.Errors.NO_ZIP);
}
_zip.entries.forEach(function (entry) {
var entryName = sanitize(targetPath, entry.entryName.toString());
var entryName = sanitize(targetPath, canonical(entry.entryName.toString()));
if (entry.isDirectory) {
Utils.makeDir(entryName);
return;
Expand Down Expand Up @@ -582,7 +584,7 @@ module.exports = function (/**String*/input) {
entries.forEach(function (entry) {
if (i <= 0) return; // Had an error already

var entryName = pth.normalize(entry.entryName.toString());
var entryName = pth.normalize(canonical(entry.entryName.toString()));

if (entry.isDirectory) {
Utils.makeDir(sanitize(targetPath, entryName));
Expand Down
59 changes: 40 additions & 19 deletions test/mocha.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,36 @@ describe('adm-zip', () => {
const files = walk(destination)

expect(files.sort()).to.deep.equal([
"./test/xxx/attributes_test/asd/New Text Document.txt",
"./test/xxx/attributes_test/blank file.txt",
"./test/xxx/attributes_test/New folder/hidden.txt",
"./test/xxx/attributes_test/New folder/hidden_readonly.txt",
"./test/xxx/attributes_test/New folder/readonly.txt",
"./test/xxx/utes_test/New folder/somefile.txt"
pth.normalize("./test/xxx/attributes_test/asd/New Text Document.txt"),
pth.normalize("./test/xxx/attributes_test/blank file.txt"),
pth.normalize("./test/xxx/attributes_test/New folder/hidden.txt"),
pth.normalize("./test/xxx/attributes_test/New folder/hidden_readonly.txt"),
pth.normalize("./test/xxx/attributes_test/New folder/readonly.txt"),
pth.normalize("./test/xxx/utes_test/New folder/somefile.txt")
].sort());
})

it('zip pathTraversal', () => {
const target = pth.join(destination, "test")
const zip = new Zip();
zip.addFile("../../../test1.ext", "content")
zip.addFile("folder/../../test2.ext", "content")
zip.addFile("test3.ext", "content")
const buf = zip.toBuffer()

const extract = new Zip(buf)
var zipEntries = zip.getEntries();
zipEntries.forEach(e => zip.extractEntryTo(e, destination, false, true));

extract.extractAllTo(target)
const files = walk(target)
expect(files.sort()).to.deep.equal([
pth.normalize('./test/xxx/test/test1.ext'),
pth.normalize('./test/xxx/test/test2.ext'),
pth.normalize('./test/xxx/test/test3.ext'),
])
})

it('zip.extractEntryTo(entry, destination, false, true)', () => {
const destination = './test/xxx'
const zip = new Zip('./test/assets/ultra.zip');
Expand All @@ -40,12 +61,12 @@ describe('adm-zip', () => {

const files = walk(destination)
expect(files.sort()).to.deep.equal([
"./test/xxx/blank file.txt",
"./test/xxx/hidden.txt",
"./test/xxx/hidden_readonly.txt",
"./test/xxx/New Text Document.txt",
"./test/xxx/readonly.txt",
"./test/xxx/somefile.txt"
pth.normalize("./test/xxx/blank file.txt"),
pth.normalize("./test/xxx/hidden.txt"),
pth.normalize("./test/xxx/hidden_readonly.txt"),
pth.normalize("./test/xxx/New Text Document.txt"),
pth.normalize("./test/xxx/readonly.txt"),
pth.normalize("./test/xxx/somefile.txt")
].sort());
})

Expand All @@ -57,12 +78,12 @@ describe('adm-zip', () => {

const files = walk(destination)
expect(files.sort()).to.deep.equal([
"./test/xxx/attributes_test/asd/New Text Document.txt",
"./test/xxx/attributes_test/blank file.txt",
"./test/xxx/attributes_test/New folder/hidden.txt",
"./test/xxx/attributes_test/New folder/hidden_readonly.txt",
"./test/xxx/attributes_test/New folder/readonly.txt",
"./test/xxx/utes_test/New folder/somefile.txt"
pth.normalize("./test/xxx/attributes_test/asd/New Text Document.txt"),
pth.normalize("./test/xxx/attributes_test/blank file.txt"),
pth.normalize("./test/xxx/attributes_test/New folder/hidden.txt"),
pth.normalize("./test/xxx/attributes_test/New folder/hidden_readonly.txt"),
pth.normalize("./test/xxx/attributes_test/New folder/readonly.txt"),
pth.normalize("./test/xxx/utes_test/New folder/somefile.txt")
].sort());
})

Expand Down Expand Up @@ -93,7 +114,7 @@ function walk(dir) {
results = results.concat(walk(file));
} else {
/* Is a file */
results.push(file);
results.push(pth.normalize(file));
}
});
return results;
Expand Down

0 comments on commit 119dcad

Please sign in to comment.