fix: escape CRLF in multipart/form-data serialization

src/common/formData.js

@@ -51,15 +51,28 @@ const isFormData = (obj) => (obj != null // neither null nor undefined
 
 const getFooter = (boundary) => `--${boundary}--\r\n\r\n`;
 
+// Escape field names and file names per the WHATWG multipart/form-data
+// serialization algorithm, preventing CR/LF (and `"`) injection into the
+// multipart body. See https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#multipart-form-data
+const escapeName = (str) => String(str)
+  .replace(/\r/g, '%0D')
+  .replace(/\n/g, '%0A')
+  .replace(/"/g, '%22');
+
+// Strip CR/LF from a part's Content-Type. Unlike a spec-compliant Blob, the
+// duck-typed blob check accepts an arbitrary `type` string, so sanitize it to
+// avoid injecting additional part headers/body into the multipart payload.
+const sanitizeContentType = (str) => String(str).replace(/[\r\n]/g, '');
+
 const getHeader = (boundary, name, field) => {
   let header = '';
 
   header += `--${boundary}\r\n`;
-  header += `Content-Disposition: form-data; name="${name}"`;
+  header += `Content-Disposition: form-data; name="${escapeName(name)}"`;
 
   if (isBlob(field)) {
-    header += `; filename="${field.name}"\r\n`;
-    header += `Content-Type: ${field.type || 'application/octet-stream'}`;
+    header += `; filename="${escapeName(field.name)}"\r\n`;
+    header += `Content-Type: ${sanitizeContentType(field.type || 'application/octet-stream')}`;
   }
 
   return `${header}\r\n\r\n`;

test/common/formData.test.js

@@ -15,6 +15,7 @@
 
 import assert from 'assert';
 import { fileURLToPath } from 'url';
+import { Readable } from 'stream';
 
 import { FormData, File, Blob } from 'formdata-node';
 // eslint-disable-next-line import/no-unresolved
@@ -57,4 +58,47 @@ describe('FormData Helpers Test', () => {
     assert.strictEqual(fds.length(), buf.length);
     assert(fds.contentType().startsWith('multipart/form-data; boundary='));
   });
+
+  it('FormDataSerializer escapes CRLF in field names, file names and blob type', async () => {
+    // A CRLF-laden blob-like value. We drive the serializer with a hand-rolled
+    // form iterable because both a spec Blob and formdata-node normalize/strip
+    // these values before our serializer would ever see them; the duck-typed
+    // isBlob() check, however, accepts an arbitrary blob-like object.
+    const blob = {
+      [Symbol.toStringTag]: 'Blob',
+      name: 'evil\r\nX-Injected: filename.txt',
+      type: 'text/plain\r\n\r\ninjected part',
+      size: 4,
+      arrayBuffer: async () => new ArrayBuffer(4),
+      stream: () => Readable.from('data'),
+      text: async () => 'data',
+      slice: () => {},
+    };
+    const form = {
+      * [Symbol.iterator]() {
+        yield ['field\r\nX-Injected: name', 'value'];
+        yield ['blob', blob];
+      },
+    };
+
+    const fds = new FormDataSerializer(form);
+    const buf = await streamToBuffer(fds.stream());
+    const boundary = fds.contentType().slice('multipart/form-data; boundary='.length);
+    const body = buf.toString();
+
+    // The tainted values must not introduce raw CRLF into the body: a real
+    // "\r\nX-Injected" / "\r\n\r\ninjected part" sequence would break out of the
+    // intended part header / body framing.
+    assert(!body.includes('\r\nX-Injected'));
+    assert(!body.includes('\r\n\r\ninjected part'));
+    // instead the CR/LF must be percent-escaped (names) or stripped (type)
+    assert(body.includes('name="field%0D%0AX-Injected: name"'));
+    assert(body.includes('filename="evil%0D%0AX-Injected: filename.txt"'));
+    assert(body.includes('Content-Type: text/plaininjected part'));
+
+    // sanity: the declared length still matches the produced body
+    assert.strictEqual(fds.length(), buf.length);
+    // and no rogue boundary was injected
+    assert.strictEqual(body.split(`--${boundary}`).length - 1, 2 /* parts */ + 1 /* footer */);
+  });
 });