CVE-2024-38372: Insufficient Information (NVD-CWE-noinfo)
value: Undici is an HTTP/1.1 client, written from scratch for Node.js. Depending on network and process conditions of a `fetch()` request, `response.arrayBuffer()` might include portion of memory from the Node.js process. This has been patched in v6.19.2.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-07-08T21:15Z
Repository: https://github.com/nodejs/undici
Language: JavaScript
Files changed: benchmarks/fetch/body-arraybuffer.mjs, lib/web/fetch/body.js, lib/web/fetch/util.js
Commit date: 2024-04-15 19:54:28 +0200
Commit message: perf: avoid unnecessary clone (#3117)

* perf(fetch): improve body mixin methods

* perf: avoid unnecessary clone
Diff: diff --git a/benchmarks/fetch/body-arraybuffer.mjs b/benchmarks/fetch/body-arraybuffer.mjs
new file mode 100644
index 000000000..f05e62dc9
--- /dev/null
+++ b/benchmarks/fetch/body-arraybuffer.mjs
@@ -0,0 +1,24 @@
+import { group, bench, run } from 'mitata'
+import { Response } from '../../lib/web/fetch/response.js'
+
+const settings = {
+  small: 2 << 8,
+  middle: 2 << 12,
+  long: 2 << 16
+}
+
+for (const [name, length] of Object.entries(settings)) {
+  const buffer = Buffer.allocUnsafe(length).map(() => (Math.random() * 100) | 0)
+  group(`${name} (length ${length})`, () => {
+    bench('Response#arrayBuffer', async () => {
+      return await new Response(buffer).arrayBuffer()
+    })
+
+    // for comparison
+    bench('Response#text', async () => {
+      return await new Response(buffer).text()
+    })
+  })
+}
+
+await run()
diff --git a/lib/web/fetch/body.js b/lib/web/fetch/body.js
index 26cce5f3e..e6af47c9a 100644
--- a/lib/web/fetch/body.js
+++ b/lib/web/fetch/body.js
@@ -307,33 +307,34 @@ function bodyMixinMethods (instance) {
         }
 
         // Return a Blob whose contents are bytes and type attribute
         // is mimeType.
         return new Blob([bytes], { type: mimeType })
-      }, instance)
+      }, instance, false)
     },
 
     arrayBuffer () {
       // The arrayBuffer() method steps are to return the result
       // of running consume body with this and the following step
       // given a byte sequence bytes: return a new ArrayBuffer
       // whose contents are bytes.
       return consumeBody(this, (bytes) => {
-        return new Uint8Array(bytes).buffer
-      }, instance)
+        // Note: arrayBuffer already cloned.
+        return bytes.buffer
+      }, instance, true)
     },
 
     text () {
       // The text() method steps are to return the result of running
       // consume body with this and UTF-8 decode.
-      return consumeBody(this, utf8DecodeBytes, instance)
+      return consumeBody(this, utf8DecodeBytes, instance, false)
     },
 
     json () {
       // The json() method steps are to return the result of running
       // consume body with this and parse JSON from bytes.
-      return consumeBody(this, parseJSONFromBytes, instance)
+      return consumeBody(this, parseJSONFromBytes, instance, false)
     },
 
     formData () {
       // The formData() method steps are to return the result of running
       // consume body with this and the following step given a byte sequence bytes:
@@ -381,11 +382,11 @@ function bodyMixinMethods (instance) {
 
         // 3. Throw a TypeError.
         throw new TypeError(
           'Content-Type was not one of "multipart/form-data" or "application/x-www-form-urlencoded".'
         )
-      }, instance)
+      }, instance, false)
     }
   }
 
   return methods
 }
@@ -397,12 +398,13 @@ function mixinBody (prototype) {
 /**
  * @see https://fetch.spec.whatwg.org/#concept-body-consume-body
  * @param {Response|Request} object
  * @param {(value: unknown) => unknown} convertBytesToJSValue
  * @param {Response|Request} instance
+ * @param {boolean} [shouldClone]
  */
-async function consumeBody (object, convertBytesToJSValue, instance) {
+async function consumeBody (object, convertBytesToJSValue, instance, shouldClone) {
   webidl.brandCheck(object, instance)
 
   // 1. If object is unusable, then return a promise rejected
   //    with a TypeError.
   if (bodyUnusable(object[kState].body)) {
@@ -430,17 +432,17 @@ async function consumeBody (object, convertBytesToJSValue, instance) {
   }
 
   // 5. If object’s body is null, then run successSteps with an
   //    empty byte sequence.
   if (object[kState].body == null) {
-    successSteps(new Uint8Array())
+    successSteps(Buffer.allocUnsafe(0))
     return promise.promise
   }
 
   // 6. Otherwise, fully read object’s body given successSteps,
   //    errorSteps, and object’s relevant global object.
-  await fullyReadBody(object[kState].body, successSteps, errorSteps)
+  await fullyReadBody(object[kState].body, successSteps, errorSteps, shouldClone)
 
   // 7. Return promise.
   return promise.promise
 }
 
diff --git a/lib/web/fetch/util.js b/lib/web/fetch/util.js
index 8c73f9096..da55a7c4d 100644
--- a/lib/web/fetch/util.js
+++ b/lib/web/fetch/util.js
@@ -1041,11 +1041,11 @@ function iteratorMixin (name, object, kInternalIterator, keyIndex = 0, valueInde
 }
 
 /**
  * @see https://fetch.spec.whatwg.org/#body-fully-read
  */
-async function fullyReadBody (body, processBody, processBodyError) {
+async function fullyReadBody (body, processBody, processBodyError, shouldClone) {
   // 1. If taskDestination is null, then set taskDestination to
   //    the result of starting a new parallel queue.
 
   // 2. Let successSteps given a byte sequence bytes be to queue a
   //    fetch task to run processBody given bytes, with taskDestination.
@@ -1067,12 +1067,11 @@ async function fullyReadBody (body, processBody, processBodyError) {
     return
   }
 
   // 5. Read all bytes from reader, given successSteps and errorSteps.
   try {
-    const result = await readAllBytes(reader)
-    successSteps(result)
+    successSteps(await readAllBytes(reader, shouldClone))
   } catch (e) {
     errorSteps(e)
   }
 }
 
@@ -1116,20 +1115,28 @@ function isomorphicEncode (input) {
 
 /**
  * @see https://streams.spec.whatwg.org/#readablestreamdefaultreader-read-all-bytes
  * @see https://streams.spec.whatwg.org/#read-loop
  * @param {ReadableStreamDefaultReader} reader
+ * @param {boolean} [shouldClone]
  */
-async function readAllBytes (reader) {
+async function readAllBytes (reader, shouldClone) {
   const bytes = []
   let byteLength = 0
 
   while (true) {
     const { done, value: chunk } = await reader.read()
 
     if (done) {
       // 1. Call successSteps with bytes.
+      if (bytes.length === 1) {
+        const { buffer, byteOffset, byteLength } = bytes[0]
+        if (shouldClone === false) {
+          return Buffer.from(buffer, byteOffset, byteLength)
+        }
+        return Buffer.from(buffer.slice(byteOffset, byteOffset + byteLength), 0, byteLength)
+      }
       return Buffer.concat(bytes, byteLength)
     }
 
     // 1. If chunk is not a Uint8Array object, call failureSteps
     //    with a TypeError and abort these steps.

Vulnerable code:       }, instance)
        return new Uint8Array(bytes).buffer
      }, instance)
      return consumeBody(this, utf8DecodeBytes, instance)
      return consumeBody(this, parseJSONFromBytes, instance)
      }, instance)
async function consumeBody (object, convertBytesToJSValue, instance) {
    successSteps(new Uint8Array())
  await fullyReadBody(object[kState].body, successSteps, errorSteps)
async function fullyReadBody (body, processBody, processBodyError) {
    const result = await readAllBytes(reader)
    successSteps(result)
async function readAllBytes (reader) {
Fixed code: import { group, bench, run } from 'mitata'
import { Response } from '../../lib/web/fetch/response.js'

const settings = {
  small: 2 << 8,
  middle: 2 << 12,
  long: 2 << 16
}

for (const [name, length] of Object.entries(settings)) {
  const buffer = Buffer.allocUnsafe(length).map(() => (Math.random() * 100) | 0)
  group(`${name} (length ${length})`, () => {
    bench('Response#arrayBuffer', async () => {
      return await new Response(buffer).arrayBuffer()
    })

    // for comparison
    bench('Response#text', async () => {
      return await new Response(buffer).text()
    })
  })
}

await run()
      }, instance, false)
        // Note: arrayBuffer already cloned.
        return bytes.buffer
      }, instance, true)
      return consumeBody(this, utf8DecodeBytes, instance, false)
      return consumeBody(this, parseJSONFromBytes, instance, false)
      }, instance, false)
 * @param {boolean} [shouldClone]
async function consumeBody (object, convertBytesToJSValue, instance, shouldClone) {
    successSteps(Buffer.allocUnsafe(0))
  await fullyReadBody(object[kState].body, successSteps, errorSteps, shouldClone)
async function fullyReadBody (body, processBody, processBodyError, shouldClone) {
    successSteps(await readAllBytes(reader, shouldClone))
 * @param {boolean} [shouldClone]
async function readAllBytes (reader, shouldClone) {
      if (bytes.length === 1) {
        const { buffer, byteOffset, byteLength } = bytes[0]
        if (shouldClone === false) {
          return Buffer.from(buffer, byteOffset, byteLength)
        }
        return Buffer.from(buffer.slice(byteOffset, byteOffset + byteLength), 0, byteLength)
      }