Package: release.debian.org Severity: normal Tags: buster User: release.debian....@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 <y...@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 <y...@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 <y...@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
