src: cleanup uv_fs_req before uv_fs_stat on existSync

RafaelGSS · RafaelGSS · commit 210e60893811 · 2025-07-08T11:14:19.000-03:00
Refs: https://hackerone.com/reports/3184178 Calling uv_fs_stat() without first calling uv_fs_req_cleanup() overwrites the pointer to the previously allocated buffer leading to a memory leak on windows PR-URL: #58915 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
diff --git a/src/node_file.cc b/src/node_file.cc
@@ -1040,6 +1040,7 @@ static void ExistsSync(const FunctionCallbackInfo<Value>& args) {
   // will **not** return an error and is therefore not enough.
   // Double check with `uv_fs_stat()`.
   if (err == 0) {
+    uv_fs_req_cleanup(&req);
     FS_SYNC_TRACE_BEGIN(stat);
     err = uv_fs_stat(nullptr, &req, path.out(), nullptr);
     FS_SYNC_TRACE_END(stat);
diff --git a/test/parallel/test-fs-existssync-memleak-longpath.js b/test/parallel/test-fs-existssync-memleak-longpath.js
@@ -0,0 +1,47 @@
+// Flags: --expose-gc --expose-internals
+'use strict';
+
+const common = require('../common');
+const { checkIfCollectableByCounting } = require('../common/gc');
+const assert = require('assert');
+const fs = require('node:fs');
+const path = require('node:path');
+const tmpdir = require('../common/tmpdir');
+const { internalBinding } = require('internal/test/binding');
+const { FSReqCallback } = internalBinding('fs');
+
+// The CVE primarily affects Windows but we should test on all platforms
+
+{
+  tmpdir.refresh();
+}
+
+{
+  const longFileNamePart = 'a'.repeat(200);
+  const fileName = tmpdir.resolve(`long-file-name-${longFileNamePart}-for-memory-leak-test.txt`);
+  fs.writeFileSync(fileName, 'test content', 'utf8');
+  const fullPath = path.resolve(fileName);
+
+  assert(fs.existsSync(fullPath), 'Test file should exist');
+
+  async function runTest() {
+    try {
+      await checkIfCollectableByCounting(
+        () => {
+          for (let i = 0; i < 10; i++) {
+            fs.existsSync(fullPath);
+          }
+          return 10;
+        },
+        FSReqCallback,
+        10
+      );
+    } catch (err) {
+      assert.ifError(err, 'Memory leak detected: FSReqCallback objects were not collected');
+    } finally {
+      tmpdir.refresh();
+    }
+  }
+
+  runTest().then(common.mustCall());
+}
