diff --git a/lib/strip-absolute-path.js b/lib/strip-absolute-path.js index 49161ddc..1aa2d2ae 100644 --- a/lib/strip-absolute-path.js +++ b/lib/strip-absolute-path.js @@ -2,13 +2,23 @@ const { isAbsolute, parse } = require('path').win32 // returns [root, stripped] +// Note that windows will think that //x/y/z/a has a "root" of //x/y, and in +// those cases, we want to sanitize it to x/y/z/a, not z/a, so we strip / +// explicitly if it's the first character. +// drive-specific relative paths on Windows get their root stripped off even +// though they are not absolute, so `c:../foo` becomes ['c:', '../foo'] module.exports = path => { let r = '' - while (isAbsolute(path)) { + + let parsed = parse(path) + while (isAbsolute(path) || parsed.root) { // windows will think that //x/y/z has a "root" of //x/y/ - const root = path.charAt(0) === '/' ? '/' : parse(path).root + // but strip the //?/C:/ off of //?/C:/path + const root = path.charAt(0) === '/' && path.slice(0, 4) !== '//?/' ? '/' + : parsed.root path = path.substr(root.length) r += root + parsed = parse(path) } return [r, path] } diff --git a/lib/unpack.js b/lib/unpack.js index 4bc2fae0..6034110d 100644 --- a/lib/unpack.js +++ b/lib/unpack.js @@ -236,13 +236,13 @@ class Unpack extends Parser { if (!this.preservePaths) { const p = normPath(entry.path) - if (p.split('/').includes('..')) { + const parts = p.split('/') + if (parts.includes('..') || isWindows && /^[a-z]:\.\.$/i.test(parts[0])) { this.warn(`path contains '..'`, p) return false } - // absolutes on posix are also absolutes on win32 - // so we only need to test this one to get both + // strip off the root const s = stripAbsolutePath(p) if (s[0]) { entry.path = s[1] @@ -255,6 +255,22 @@ class Unpack extends Parser { else entry.absolute = normPath(path.resolve(this.cwd, entry.path)) + // if we somehow ended up with a path that escapes the cwd, and we are + // not in preservePaths mode, then something is fishy! This should have + // been prevented above, so ignore this for coverage. + /* istanbul ignore if - defense in depth */ + if (!this.preservePaths && + entry.absolute.indexOf(this.cwd + '/') !== 0 && + entry.absolute !== this.cwd) { + this.warn('TAR_ENTRY_ERROR', 'path escaped extraction target', { + entry, + path: normPath(entry.path), + resolvedPath: entry.absolute, + cwd: this.cwd, + }) + return false + } + // an archive can set properties on the extraction directory, but it // may not replace the cwd with a different kind of thing entirely. if (entry.absolute === this.cwd && diff --git a/test/strip-absolute-path.js b/test/strip-absolute-path.js index beb057ff..c50e2858 100644 --- a/test/strip-absolute-path.js +++ b/test/strip-absolute-path.js @@ -1,14 +1,52 @@ const t = require('tap') const stripAbsolutePath = require('../lib/strip-absolute-path.js') +const cwd = process.cwd() +const requireInject = require('require-inject') -const cases = { - '/': ['/', ''], - '////': ['////', ''], - 'c:///a/b/c': ['c:///', 'a/b/c'], - '\\\\foo\\bar\\baz': ['\\\\foo\\bar\\', 'baz'], - '//foo//bar//baz': ['//', 'foo//bar//baz'], - 'c:\\c:\\c:\\c:\\\\d:\\e/f/g': ['c:\\c:\\c:\\c:\\\\d:\\', 'e/f/g'], -} +t.test('basic', t => { + const cases = { + '/': ['/', ''], + '////': ['////', ''], + 'c:///a/b/c': ['c:///', 'a/b/c'], + '\\\\foo\\bar\\baz': ['\\\\foo\\bar\\', 'baz'], + '//foo//bar//baz': ['//', 'foo//bar//baz'], + 'c:\\c:\\c:\\c:\\\\d:\\e/f/g': ['c:\\c:\\c:\\c:\\\\d:\\', 'e/f/g'], + } -for (const [input, [root, stripped]] of Object.entries(cases)) - t.strictSame(stripAbsolutePath(input), [root, stripped], input) + for (const [input, [root, stripped]] of Object.entries(cases)) + t.strictSame(stripAbsolutePath(input, cwd), [root, stripped], input) + t.end() +}) + +t.test('drive-local paths', t => { + const env = process.env + t.teardown(() => process.env = env) + const cwd = 'D:\\safety\\land' + const realPath = require('path') + // be windowsy + const path = { + ...realPath.win32, + win32: realPath.win32, + posix: realPath.posix, + } + const stripAbsolutePath = requireInject('../lib/strip-absolute-path.js', { path }) + const cases = { + '/': ['/', ''], + '////': ['////', ''], + 'c:///a/b/c': ['c:///', 'a/b/c'], + '\\\\foo\\bar\\baz': ['\\\\foo\\bar\\', 'baz'], + '//foo//bar//baz': ['//', 'foo//bar//baz'], + 'c:\\c:\\c:\\c:\\\\d:\\e/f/g': ['c:\\c:\\c:\\c:\\\\d:\\', 'e/f/g'], + 'c:..\\system\\explorer.exe': ['c:', '..\\system\\explorer.exe'], + 'd:..\\..\\unsafe\\land': ['d:', '..\\..\\unsafe\\land'], + 'c:foo': ['c:', 'foo'], + 'D:mark': ['D:', 'mark'], + '//?/X:/y/z': ['//?/X:/', 'y/z'], + '\\\\?\\X:\\y\\z': ['\\\\?\\X:\\', 'y\\z'], + } + for (const [input, [root, stripped]] of Object.entries(cases)) { + if (!t.strictSame(stripAbsolutePath(input, cwd), [root, stripped], input)) + break + } + t.end() +})