CVE-2021-32804: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: The npm package "tar" (aka node-tar) before versions 6.1.1, 5.0.6, 4.4.14, and 3.3.2 has a arbitrary File Creation/Overwrite vulnerability due to insufficient absolute path sanitization. node-tar aims to prevent extraction of absolute file paths by turning absolute paths into relative paths when the `preservePaths` flag is not set to `true`. This is achieved by stripping the absolute path root from any absolute file paths contained in a tar file. For example `/home/user/.bashrc` would turn into `home/user/.bashrc`. This logic was insufficient when file paths contained repeated path roots such as `////home/user/.bashrc`. `node-tar` would only strip a single path root from such paths. When given an absolute file path with repeating path roots, the resulting path (e.g. `///home/user/.bashrc`) would still resolve to an absolute path, thus allowing arbitrary file creation and overwrite. This issue was addressed in releases 3.2.2, 4.4.14, 5.0.6 and 6.1.1. Users may work around this vulnerability without upgrading by creating a custom `onentry` method which sanitizes the `entry.path` or a `filter` method which removes entries with absolute paths. See referenced GitHub Advisory for details. Be aware of CVE-2021-32803 which fixes a similar bug in later versions of tar.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 8.1
Published: 2021-08-03T19:15Z
Repository: https://github.com/npm/node-tar
Language: JavaScript
Files changed: lib/strip-absolute-path.js, lib/unpack.js, lib/write-entry.js, test/strip-absolute-path.js, test/unpack.js, test/write-entry.js
Commit date: 2021-07-23 15:27:25 -0700
Commit message: fix: strip absolute paths more comprehensively

Diff: diff --git a/lib/strip-absolute-path.js b/lib/strip-absolute-path.js
new file mode 100644
index 0000000..49161dd
--- /dev/null
+++ b/lib/strip-absolute-path.js
@@ -0,0 +1,14 @@
+// unix absolute paths are also absolute on win32, so we use this for both
+const { isAbsolute, parse } = require('path').win32
+
+// returns [root, stripped]
+module.exports = path => {
+  let r = ''
+  while (isAbsolute(path)) {
+    // windows will think that //x/y/z has a "root" of //x/y/
+    const root = path.charAt(0) === '/' ? '/' : parse(path).root
+    path = path.substr(root.length)
+    r += root
+  }
+  return [r, path]
+}
diff --git a/lib/unpack.js b/lib/unpack.js
index 7d4b79d..216fa71 100644
--- a/lib/unpack.js
+++ b/lib/unpack.js
@@ -12,10 +12,11 @@ const fs = require('fs')
 const fsm = require('fs-minipass')
 const path = require('path')
 const mkdir = require('./mkdir.js')
 const wc = require('./winchars.js')
 const pathReservations = require('./path-reservations.js')
+const stripAbsolutePath = require('./strip-absolute-path.js')
 
 const ONENTRY = Symbol('onEntry')
 const CHECKFS = Symbol('checkFs')
 const CHECKFS2 = Symbol('checkFs2')
 const ISREUSABLE = Symbol('isReusable')
@@ -222,15 +223,14 @@ class Unpack extends Parser {
         return false
       }
 
       // absolutes on posix are also absolutes on win32
       // so we only need to test this one to get both
-      if (path.win32.isAbsolute(p)) {
-        const parsed = path.win32.parse(p)
-        entry.path = p.substr(parsed.root.length)
-        const r = parsed.root
-        this.warn('TAR_ENTRY_INFO', `stripping ${r} from absolute path`, {
+      const [root, stripped] = stripAbsolutePath(p)
+      if (root) {
+        entry.path = stripped
+        this.warn('TAR_ENTRY_INFO', `stripping ${root} from absolute path`, {
           entry,
           path: p,
         })
       }
     }
diff --git a/lib/write-entry.js b/lib/write-entry.js
index 1d0b746..0301759 100644
--- a/lib/write-entry.js
+++ b/lib/write-entry.js
@@ -21,10 +21,11 @@ const OPENFILE = Symbol('openfile')
 const ONOPENFILE = Symbol('onopenfile')
 const CLOSE = Symbol('close')
 const MODE = Symbol('mode')
 const warner = require('./warn-mixin.js')
 const winchars = require('./winchars.js')
+const stripAbsolutePath = require('./strip-absolute-path.js')
 
 const modeFix = require('./mode-fix.js')
 
 const WriteEntry = warner(class WriteEntry extends MiniPass {
   constructor (p, opt) {
@@ -50,16 +51,16 @@ const WriteEntry = warner(class WriteEntry extends MiniPass {
 
     if (typeof opt.onwarn === 'function')
       this.on('warn', opt.onwarn)
 
     let pathWarn = false
-    if (!this.preservePaths && path.win32.isAbsolute(p)) {
-      // absolutes on posix are also absolutes on win32
-      // so we only need to test this one to get both
-      const parsed = path.win32.parse(p)
-      this.path = p.substr(parsed.root.length)
-      pathWarn = parsed.root
+    if (!this.preservePaths) {
+      const [root, stripped] = stripAbsolutePath(this.path)
+      if (root) {
+        this.path = stripped
+        pathWarn = root
+      }
     }
 
     this.win32 = !!opt.win32 || process.platform === 'win32'
     if (this.win32) {
       this.path = winchars.decode(this.path.replace(/\\/g, '/'))
@@ -349,14 +350,16 @@ const WriteEntryTar = warner(class WriteEntryTar extends MiniPass {
 
     if (typeof opt.onwarn === 'function')
       this.on('warn', opt.onwarn)
 
     let pathWarn = false
-    if (path.isAbsolute(this.path) && !this.preservePaths) {
-      const parsed = path.parse(this.path)
-      pathWarn = parsed.root
-      this.path = this.path.substr(parsed.root.length)
+    if (!this.preservePaths) {
+      const [root, stripped] = stripAbsolutePath(this.path)
+      if (root) {
+        this.path = stripped
+        pathWarn = root
+      }
     }
 
     this.remain = readEntry.size
     this.blockRemain = readEntry.startBlockSize
 
diff --git a/test/strip-absolute-path.js b/test/strip-absolute-path.js
new file mode 100644
index 0000000..beb057f
--- /dev/null
+++ b/test/strip-absolute-path.js
@@ -0,0 +1,14 @@
+const t = require('tap')
+const stripAbsolutePath = require('../lib/strip-absolute-path.js')
+
+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)
diff --git a/test/unpack.js b/test/unpack.js
index cc3538a..1b2a033 100644
--- a/test/unpack.js
+++ b/test/unpack.js
@@ -774,18 +774,21 @@ t.test('absolute paths', t => {
     rimraf.sync(dir)
     mkdirp.sync(dir)
   })
 
   const absolute = path.resolve(dir, 'd/i/r/absolute')
+  const root = path.parse(absolute).root
+  const extraAbsolute = root + root + root + absolute
+  t.ok(path.isAbsolute(extraAbsolute))
   t.ok(path.isAbsolute(absolute))
   const parsed = path.parse(absolute)
   const relative = absolute.substr(parsed.root.length)
   t.notOk(path.isAbsolute(relative))
 
   const data = makeTar([
     {
-      path: absolute,
+      path: extraAbsolute,
       type: 'File',
       size: 1,
       atime: new Date('1979-07-01T19:10:00.000Z'),
       ctime: new Date('2011-03-27T22:16:31.000Z'),
       mtime: new Date('2011-03-27T22:16:31.000Z'),
@@ -796,11 +799,11 @@ t.test('absolute paths', t => {
   ])
 
   t.test('warn and correct', t => {
     const check = t => {
       t.match(warnings, [[
-        'stripping / from absolute path',
+        `stripping ${root}${root}${root}${root} from absolute path`,
         { path: absolute, code: 'TAR_ENTRY_INFO' },
       ]])
       t.ok(fs.lstatSync(path.resolve(dir, relative)).isFile(), 'is file')
       t.end()
     }
diff --git a/test/write-entry.js b/test/write-entry.js
index 89ddcc8..c177f89 100644
--- a/test/write-entry.js
+++ b/test/write-entry.js
@@ -383,11 +383,14 @@ t.test('nonexistent file', t => {
     t.end()
   })
 })
 
 t.test('absolute path', t => {
-  const f = path.resolve(files, '512-bytes.txt')
+  const absolute = path.resolve(files, '512-bytes.txt')
+  const { root } = path.parse(absolute)
+  const f = root + root + root + absolute
+  const warn = root + root + root + root
   t.test('preservePaths=false strict=false', t => {
     const warnings = []
     const ws = new WriteEntry(f, {
       cwd: files,
       onwarn: (c, m, p) => warnings.push([c, m, p]),
@@ -396,17 +399,17 @@ t.test('absolute path', t => {
     ws.on('data', c => out.push(c))
     ws.on('end', _ => {
       out = Buffer.concat(out)
       t.equal(out.length, 1024)
       t.match(warnings, [[
-        'TAR_ENTRY_INFO', /stripping .* from absolute path/, { path: f },
+        'TAR_ENTRY_INFO', `stripping ${warn} from absolute path`, { path: f },
       ]])
 
       t.match(ws.header, {
         cksumValid: true,
         needPax: false,
-        path: f.replace(/^(\/|[a-z]:\\\\)/, ''),
+        path: f.replace(/^(\/|[a-z]:\\\\){4}/, ''),
         mode: 0o644,
         size: 512,
         linkpath: null,
         uname: 'isaacs',
         gname: null,

Vulnerable code:       if (path.win32.isAbsolute(p)) {
        const parsed = path.win32.parse(p)
        entry.path = p.substr(parsed.root.length)
        const r = parsed.root
        this.warn('TAR_ENTRY_INFO', `stripping ${r} from absolute path`, {
    if (!this.preservePaths && path.win32.isAbsolute(p)) {
      // absolutes on posix are also absolutes on win32
      // so we only need to test this one to get both
      const parsed = path.win32.parse(p)
      this.path = p.substr(parsed.root.length)
      pathWarn = parsed.root
    if (path.isAbsolute(this.path) && !this.preservePaths) {
      const parsed = path.parse(this.path)
      pathWarn = parsed.root
      this.path = this.path.substr(parsed.root.length)
      path: absolute,
        'stripping / from absolute path',
  const f = path.resolve(files, '512-bytes.txt')
        'TAR_ENTRY_INFO', /stripping .* from absolute path/, { path: f },
        path: f.replace(/^(\/|[a-z]:\\\\)/, ''),
Fixed code: // unix absolute paths are also absolute on win32, so we use this for both
const { isAbsolute, parse } = require('path').win32

// returns [root, stripped]
module.exports = path => {
  let r = ''
  while (isAbsolute(path)) {
    // windows will think that //x/y/z has a "root" of //x/y/
    const root = path.charAt(0) === '/' ? '/' : parse(path).root
    path = path.substr(root.length)
    r += root
  }
  return [r, path]
}
const stripAbsolutePath = require('./strip-absolute-path.js')
      const [root, stripped] = stripAbsolutePath(p)
      if (root) {
        entry.path = stripped
        this.warn('TAR_ENTRY_INFO', `stripping ${root} from absolute path`, {
const stripAbsolutePath = require('./strip-absolute-path.js')
    if (!this.preservePaths) {
      const [root, stripped] = stripAbsolutePath(this.path)
      if (root) {
        this.path = stripped
        pathWarn = root
      }
    if (!this.preservePaths) {
      const [root, stripped] = stripAbsolutePath(this.path)
      if (root) {
        this.path = stripped
        pathWarn = root
      }
const t = require('tap')
const stripAbsolutePath = require('../lib/strip-absolute-path.js')

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)
  const root = path.parse(absolute).root
  const extraAbsolute = root + root + root + absolute
  t.ok(path.isAbsolute(extraAbsolute))
      path: extraAbsolute,
        `stripping ${root}${root}${root}${root} from absolute path`,
  const absolute = path.resolve(files, '512-bytes.txt')
  const { root } = path.parse(absolute)
  const f = root + root + root + absolute
  const warn = root + root + root + root
        'TAR_ENTRY_INFO', `stripping ${warn} from absolute path`, { path: f },
        path: f.replace(/^(\/|[a-z]:\\\\){4}/, ''),