Arbitrary file write in node-tar through a stale directory cache

Extracting an untrusted tar archive looks like file copying. It isn't. A tar file is a list of entries with names attached, and the names are attacker-controlled strings. Every extractor has to decide what it will refuse: absolute paths, .. components, and the awkward one, symbolic links.

Say the archive writes a symlink called lib pointing at /etc, then writes an entry called lib/passwd. Neither name contains a .., and the second write still lands outside the extraction directory, because the kernel resolved the link on the way through.

node-tar states the guarantee it's trying to make, which is more than most:

node-tar aims to guarantee that any file whose location would be modified by a symbolic link is not extracted. This is, in part, achieved by ensuring that extracted directories are not symlinks.

That matters more here than in most tar libraries, because npm depends on it. The npm 7 CLI lists tar in its dependencies, so this code runs on every package tarball anybody installs. chen-robert, ginkoid and levpachmanov found two ways to defeat the guarantee, and both of them are about strings rather than about files.

node-tar keeps that promise by building each directory itself, one component at a time, with fs.mkdir. When mkdir fails it does an lstat on the component to find out what's actually there. A directory is fine, keep walking. A symlink is not:

} else if (st.isSymbolicLink())
  return cb(new SymlinkError(part, part + '/' + parts.join('/')))

That's the check. It's the only one. Which brings us to the cache.

The directory cache#

Doing that walk for every entry is a lot of syscalls. An archive with a thousand files in deep/nested/dir/ would mkdir and lstat the same three components a thousand times. So node-tar keeps a Map of paths it has already created, dirCache, and the first thing mkdir.js does is check it:

if (cache && cache.get(dir) === true)
  return done()

A cache hit skips the walk. It also skips the lstat, and therefore the symlink check. The cache does not sit beside the security check. It replaces it. Every entry asserts that a real directory is at that path right now.

The extractor knows this, and has code to keep the assertion true. Before it writes any non-directory entry it prunes the cache of anything that entry is about to clobber:

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)
  }
}

Directory x gets created and cached. A symlink entry, also called x, arrives. The loop deletes /cwd/x from the cache, node-tar lstats the path, sees a directory where a symlink is about to go, rmdirs it and creates the link. Later entries under x/ now go through mkdir, hit the lstat, find the symlink, and get refused. That works.

It works as long as the string in the cache and the string the operating system will resolve are the same string. Two ways they aren't.

FOO and foo#

The three of them are credited on the GitHub advisory, GHSA-9r2w-394v-53qc, published as CVE-2021-37701. The second of their two bypasses is the one you can follow all the way through in your head.

Take a filesystem that's case-insensitive, which is to say a default macOS install or any Windows machine. Now an archive:

FOO/                 directory
foo -> /tmp/target   symlink
FOO/payload          file

Entry one creates FOO and puts /cwd/FOO in the cache. Entry two is not a directory, so the prune loop runs, comparing /cwd/foo against the cache keys. /cwd/FOO is not equal to /cwd/foo, and doesn't start with /cwd/foo/, so it stays. Then the extractor lstats /cwd/foo, and on this filesystem that's the directory it made a moment ago, so it rmdirs it and writes the symlink. The directory is gone. The cache entry claiming it exists is not.

Entry three asks for /cwd/FOO/payload. node-tar calls mkdir for the parent, /cwd/FOO, gets a cache hit, returns immediately, and opens the file. The kernel resolves FOO to foo, foo to /tmp/target, and the payload is written outside the extraction directory. The advisory's phrasing: the subsequent entry is "placed in the target of the symbolic link, thinking that the directory had already been created."

Backslashes on POSIX#

The other bypass involves a character that means two different things depending on who's reading it. From the advisory:

The cache checking logic used both \ and / characters as path separators, however \ is a valid filename character on posix systems.

You can see both conventions in the pre-fix code. The prune loop above treats entry.absolute + '\\' as a prefix worth matching. So does the directory builder, which splits the path it's been asked to create on either character:

const sub = path.relative(cwd, dir)
const parts = sub.split(/\/|\\/)

On Linux that's just wrong. Asked to ensure a\b exists, node-tar splits it into a and b, creates two real nested directories, walks them, lstats them, finds them satisfactory and reports success. Then the entry gets written to the literal path a\b, one directory name, which the kernel looks up as a single component that node-tar never checked. The path the safety check walked and the path the write goes to are not the same path. Now add a symlink, and a directory whose name collides under one convention but not the other. The cache vouches for something that is no longer there. That is the case-insensitive bug again, in a second spelling.

The fix is two lines of policy applied everywhere. From the advisory:

1. All paths are normalized to use / as a path separator, replacing \ with / on Windows systems,

and leaving \ intact in the path on posix systems. This is performed in depth, at every level of the program where paths are consumed.

1. Directory cache pruning is performed case-insensitively.

The first is a normPath helper. It does nothing on POSIX. On Windows it runs replace(/\\/g, '/'), and it is applied to every path entering or leaving the extractor. The separator split becomes a plain sub.split('/'). The second is a five-line function:

const pruneCache = (cache, abs) => {
  // clear the cache if it's a case-insensitive match, since we can't
  // know if the current file system is case-sensitive or not.
  abs = normPath(abs).toLowerCase()
  for (const path of cache.keys()) {
    const plower = path.toLowerCase()
    if (plower === abs || plower.toLowerCase().indexOf(abs + '/') === 0)
      cache.delete(path)
  }
}

Note the reasoning in the comment. There is no portable way to ask a filesystem whether it is case-sensitive. The answer can differ per mount point on one machine. So the cache stops trying to be right and becomes pessimistic instead. The advisory shrugs at the cost: "This may result in undue cache misses on case-sensitive file systems, but the performance impact is negligible."

Fixed in 4.4.16, 5.0.8 and 6.1.7, against vulnerable ranges of >= 3.0.0, < 4.4.16, >= 5.0.0, < 5.0.8 and >= 6.0.0, < 6.1.7. The advisory also says the v3 branch is deprecated and didn't get patches of its own, and tells v3 users to move to a newer release. For anyone stuck, the suggested workaround is to pass a filter to tar.x that returns false for entry.type === 'SymbolicLink', with the maintainers noting that you should upgrade "rather than attempt to sanitize tar input themselves."

Two more advisories followed a couple of releases later, and they're the same shape with different string comparisons. CVE-2021-37712 covers two more spellings. One is Unicode that normalises to the same value. The other is Windows 8.3 short paths, where MICROS~1 and the long name it abbreviates are one file to the filesystem and two keys to the cache. CVE-2021-37713 is Windows drive-relative paths such as C:some\path, which path.resolve resolves against the current directory of the C: drive instead of the extraction target. Both landed in 4.4.18, 5.0.10 and 6.1.9, so the version numbers don't line up with the first set.