--- Begin Message ---
- To: Debian Bug Tracking System <submit@bugs.debian.org>
- Subject: buster-pu: package node-tar/4.4.6+ds1-3+deb10u1
- From: Yadd <yadd@debian.org>
- Date: Thu, 12 Aug 2021 00:11:44 +0200
- Message-id: <162871990464.2945443.15550455421089215339.reportbug@deb007.xnr.fr>
Package: release.debian.org
Severity: normal
Tags: buster
User: release.debian.org@packages.debian.org
Usertags: pu
[ Reason ]
node-tar is vulnerable to 2 CVE:
* #992110, CVE-2021-32803: arbitrary File Creation/Overwrite
vulnerability via insufficient symlink protection
* #992111, CVE-2021-32804: arbitrary File Creation/Overwrite
vulnerability due to insufficient absolute path sanitization
[ Impact ]
2 medium vulnerabilities
[ Tests ]
Test not launched in Buster
[ Risks ]
Low risk: test passed and upstream patch applied with minor changes
[ Checklist ]
[X] *all* changes are documented in the d/changelog
[X] I reviewed all changes and I approve them
[X] attach debdiff against the package in (old)stable
[X] the issue is verified as fixed in unstable
[ Changes ]
Add new checks
Cheers,
Yadd
diff --git a/debian/changelog b/debian/changelog
index 83bacd9..8b3a42d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,13 @@
+node-tar (4.4.6+ds1-3+deb10u1) buster; urgency=medium
+
+ * Team upload
+ * Remove paths from dirCache when no longer dirs
+ (Closes: #992110, CVE-2021-32803)
+ * Strip absolute paths more comprehensively
+ (Closes: #992111, CVE-2021-32804)
+
+ -- Yadd <yadd@debian.org> Thu, 12 Aug 2021 00:06:36 +0200
+
node-tar (4.4.6+ds1-3) unstable; urgency=medium
* Team upload
diff --git a/debian/patches/CVE-2021-32803.patch b/debian/patches/CVE-2021-32803.patch
new file mode 100644
index 0000000..44e29a4
--- /dev/null
+++ b/debian/patches/CVE-2021-32803.patch
@@ -0,0 +1,106 @@
+Description: Remove paths from dirCache when no longer dirs
+Author: isaacs <i@izs.me>
+Origin: upstream, https://github.com/npm/node-tar/commit/46fe3508
+Bug: https://github.com/npm/node-tar/security/advisories/GHSA-r628-mhmh-qjhw
+Bug-Debian: https://bugs.debian.org/992110
+Forwarded: not-needed
+Reviewed-By: Yadd <yadd@debian.org>
+Last-Update: 2021-08-11
+
+--- a/lib/unpack.js
++++ b/lib/unpack.js
+@@ -407,6 +407,20 @@
+ // check if a thing is there, and if so, try to clobber it
+ [CHECKFS] (entry) {
+ this[PEND]()
++
++ // if we are not creating a directory, and the path is in the dirCache,
++ // then that means we are about to delete the directory we created
++ // previously, and it is no longer going to be a directory, and neither
++ // is any of its children.
++ if (entry.type !== 'Directory') {
++ for (const path of this.dirCache.keys()) {
++ if (path === entry.absolute ||
++ path.indexOf(entry.absolute + '/') === 0 ||
++ path.indexOf(entry.absolute + '\\') === 0)
++ this.dirCache.delete(path)
++ }
++ }
++
+ this[MKDIR](path.dirname(entry.absolute), this.dmode, er => {
+ if (er)
+ return this[ONERROR](er, entry)
+@@ -468,6 +482,15 @@
+ }
+
+ [CHECKFS] (entry) {
++ if (entry.type !== 'Directory') {
++ for (const path of this.dirCache.keys()) {
++ if (path === entry.absolute ||
++ path.indexOf(entry.absolute + '/') === 0 ||
++ path.indexOf(entry.absolute + '\\') === 0)
++ this.dirCache.delete(path)
++ }
++ }
++
+ const er = this[MKDIR](path.dirname(entry.absolute), this.dmode)
+ if (er)
+ return this[ONERROR](er, entry)
+--- a/test/unpack.js
++++ b/test/unpack.js
+@@ -2417,3 +2417,55 @@
+
+ t.end()
+ })
++
++t.test('drop entry from dirCache if no longer a directory', t => {
++ const dir = path.resolve(unpackdir, 'dir-cache-error')
++ mkdirp.sync(dir + '/sync/y')
++ mkdirp.sync(dir + '/async/y')
++ const data = makeTar([
++ {
++ path: 'x',
++ type: 'Directory',
++ },
++ {
++ path: 'x',
++ type: 'SymbolicLink',
++ linkpath: './y',
++ },
++ {
++ path: 'x/ginkoid',
++ type: 'File',
++ size: 'ginkoid'.length,
++ },
++ 'ginkoid',
++ '',
++ '',
++ ])
++ t.plan(2)
++ const WARNINGS = {}
++ const check = (t, path) => {
++ t.equal(fs.statSync(path + '/x').isDirectory(), true)
++ t.equal(fs.lstatSync(path + '/x').isSymbolicLink(), true)
++ t.equal(fs.statSync(path + '/y').isDirectory(), true)
++ t.strictSame(fs.readdirSync(path + '/y'), [])
++ t.throws(() => fs.readFileSync(path + '/x/ginkoid'), { code: 'ENOENT' })
++ t.strictSame(WARNINGS[path], [
++ 'Cannot extract through symbolic link',
++ ])
++ t.end()
++ }
++ t.test('async', t => {
++ const path = dir + '/async'
++ new Unpack({ cwd: path })
++ .on('warn', (msg) => WARNINGS[path] = [msg])
++ .on('end', () => check(t, path))
++ .end(data)
++ })
++ t.test('sync', t => {
++ const path = dir + '/sync'
++ new UnpackSync({ cwd: path })
++ .on('warn', (msg) => WARNINGS[path] = [msg])
++ .end(data)
++ check(t, path)
++ })
++})
diff --git a/debian/patches/CVE-2021-32804.patch b/debian/patches/CVE-2021-32804.patch
new file mode 100644
index 0000000..d8b23f4
--- /dev/null
+++ b/debian/patches/CVE-2021-32804.patch
@@ -0,0 +1,150 @@
+Description: strip absolute paths more comprehensively
+Author: isaacs <i@izs.me>
+Origin: upstream, https://github.com/npm/node-tar/commit/efc6bb0d
+Bug: https://github.com/npm/node-tar/security/advisories/GHSA-3jfq-g458-7qm9
+Bug-Debian: https://bugs.debian.org/992111
+Forwarded: not-needed
+Reviewed-By: Yadd <yadd@debian.org>
+Last-Update: 2021-08-11
+
+--- /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]
++}
+--- a/lib/unpack.js
++++ b/lib/unpack.js
+@@ -9,6 +9,7 @@
+ const mkdir = require('./mkdir.js')
+ const mkdirSync = mkdir.sync
+ const wc = require('./winchars.js')
++const stripAbsolutePath = require('./strip-absolute-path.js')
+
+ const ONENTRY = Symbol('onEntry')
+ const CHECKFS = Symbol('checkFs')
+@@ -189,10 +190,10 @@
+
+ // 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)
+- this.warn('stripping ' + parsed.root + ' from absolute path', p)
+- entry.path = p.substr(parsed.root.length)
++ const s = stripAbsolutePath(p)
++ if (s[0]) {
++ entry.path = s[1]
++ this.warn(`stripping ${s[0]} from absolute path`, p)
+ }
+ }
+
+--- a/lib/write-entry.js
++++ b/lib/write-entry.js
+@@ -26,6 +26,7 @@
+ 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')
+
+@@ -54,12 +55,12 @@
+ if (typeof opt.onwarn === 'function')
+ this.on('warn', opt.onwarn)
+
+- 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.warn('stripping ' + parsed.root + ' from absolute path', p)
+- this.path = p.substr(parsed.root.length)
++ if (!this.preservePaths) {
++ const s = stripAbsolutePath(this.path)
++ if (s[0]) {
++ this.path = s[1]
++ this.warn('stripping ' + s[0] + ' from absolute path', p)
++ }
+ }
+
+ this.win32 = !!opt.win32 || process.platform === 'win32'
+@@ -333,13 +334,15 @@
+ if (typeof opt.onwarn === 'function')
+ this.on('warn', opt.onwarn)
+
+- if (path.isAbsolute(this.path) && !this.preservePaths) {
+- const parsed = path.parse(this.path)
+- this.warn(
+- 'stripping ' + parsed.root + ' from absolute path',
+- this.path
+- )
+- this.path = this.path.substr(parsed.root.length)
++ if (!this.preservePaths) {
++ const s = stripAbsolutePath(this.path)
++ if (s[0]) {
++ this.warn(
++ 'stripping ' + s[0] + ' from absolute path',
++ this.path
++ )
++ this.path = s[1]
++ }
+ }
+
+ this.remain = readEntry.size
+--- /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)
+--- a/test/write-entry.js
++++ b/test/write-entry.js
+@@ -360,7 +360,10 @@
+ })
+
+ 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, {
+@@ -373,13 +376,13 @@
+ out = Buffer.concat(out)
+ t.equal(out.length, 1024)
+ t.match(warnings, [[
+- /stripping .* from absolute path/, f
++ 'stripping ' + warn + ' from absolute 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,
diff --git a/debian/patches/series b/debian/patches/series
index 0b8149b..6aa9174 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,2 +1,4 @@
noneed-chmodr-tests.patch
api-backward-compatibility.patch
+CVE-2021-32803.patch
+CVE-2021-32804.patch
--- End Message ---