Skip to content

Comments

fix(server): Uncontrolled data used in path expression#1101

Merged
cure53 merged 1 commit intocure53:mainfrom
odaysec:patch-1
May 14, 2025
Merged

fix(server): Uncontrolled data used in path expression#1101
cure53 merged 1 commit intocure53:mainfrom
odaysec:patch-1

Conversation

@odaysec
Copy link
Contributor

@odaysec odaysec commented May 13, 2025

fs.createReadStream(filename).pipe(res);

To fix the issue, we need to ensure that the constructed file path is safe and does not allow access to files outside the intended directory. This can be achieved by:

  1. Normalizing the path using path.resolve to remove any .. segments.
  2. Using fs.realpathSync to resolve symbolic links.
  3. Verifying that the resolved path starts with the intended root directory (in this case, process.cwd()).

If the resolved path does not start with the root directory, the request should be rejected with an appropriate HTTP status code (e.g., 403 Forbidden).


Accessing files using paths constructed from user-controlled data can allow an attacker to access unexpected resources. This can result in sensitive information being revealed or deleted, or an attacker being able to influence behavior by modifying unexpected files.

POC

In the first (bad) the code reads the file name from an HTTP request, then accesses that file within a root folder. A malicious user could enter a file name containing "../" segments to navigate outside the root folder and access sensitive files.

const fs = require('fs'),
      http = require('http'),
      url = require('url');

const ROOT = "/var/www/";

var server = http.createServer(function(req, res) {
  let filePath = url.parse(req.url, true).query.path;

  // BAD: This function uses unsanitized input that can read any file on the file system.
  res.write(fs.readFileSync(ROOT + filePath, 'utf8'));
});

The second (good) example shows how to avoid access to sensitive files by sanitizing the file path. First, the code resolves the file name relative to a root folder, normalizing the path and removing any "../" segments in the process. Then, the code calls fs.realpathSync to resolve any symbolic links in the path. Finally, the code checks that the normalized path starts with the path of the root folder, ensuring the file is contained within the root folder.

const fs = require('fs'),
      http = require('http'),
      path = require('path'),
      url = require('url');

const ROOT = "/var/www/";

var server = http.createServer(function(req, res) {
  let filePath = url.parse(req.url, true).query.path;

  // GOOD: Verify that the file path is under the root directory
  filePath = fs.realpathSync(path.resolve(ROOT, filePath));
  if (!filePath.startsWith(ROOT)) {
    res.statusCode = 403;
    res.end();
    return;
  }
  res.write(fs.readFileSync(filePath, 'utf8'));
});

References

Path Traversal
sanitize-filename package
CWE-22
CWE-23
CWE-36
CWE-73
CWE-99

Dependencies

  • Resolved dependency
  • Open dependency

@cure53 cure53 merged commit 6bc6d60 into cure53:main May 14, 2025
8 checks passed
@cure53
Copy link
Owner

cure53 commented May 16, 2025

Not a vulnerability, the CVE was created by the "finder" without any coordination and should be deleted.

@sachin06192
Copy link

@cure53 When can we expect a release of this patch ? NexusIQ scans flagging this as a high finding.

@cure53
Copy link
Owner

cure53 commented May 19, 2025

It's an invalid finding, the CVE was revoked last week.

@shyam-sathyan
Copy link

shyam-sathyan commented May 19, 2025

@cure53 CVE-2025-48050 is reported. Is this fixed now?

@cure53
Copy link
Owner

cure53 commented May 19, 2025

This was never a valid finding, however we will release 3.2.6. soon for other reasons.

The CVE was released without coordination, and we had to clean that mess up. Please direct any feedback to @odaysec who decided to file the non-sense CVE and thereby cause all the kerfuffle.

@gogo2464
Copy link

Hello. I would like to reproduce the vulnerability in order to proove it presence.

I can not go out of process.cwd() in:

filename = path.join(process.cwd(), uri);

When I do

http://localhost:8000/../../README.md

It stops the path of ../ at the currrent directory.

and when I do:

http://localhost:8000//etc/passwd

it does not directly goes to /etc/password

I debugged with:

/* jshint globalstrict:true, node:true */
'use strict';

var http = require('http'),
  url = require('url'),
  path = require('path'),
  fs = require('fs'),
  domain = require('domain').create();

var mimeTypes = {
  html: 'text/html',
  json: 'application/json',
  js: 'text/javascript',
  css: 'text/css',
};

// *VERY* minimal static file server
http
  .createServer(function (req, res) {
    var uri = url.parse(req.url).pathname,
      filename;

    if (uri === '/test/') {
      uri = '/test/index.html';
    }
    filename = path.join(process.cwd(), uri);

    console.log(filename);
    console.log("uri: " + uri);

   console.log(path.join(process.cwd(), '/etc/passwd'));


    domain.on('error', function () {
      res.writeHead(404, { 'Content-Type': 'text/plain' });
      res.end('404 Not Found\n');
    });
    domain.run(function () {
      var mimeType =
        mimeTypes[path.extname(filename).split('.')[1]] || 'text/plain';
      res.writeHead(200, { 'Content-Type': mimeType });
      fs.createReadStream(filename).pipe(res);
    });
  })
  .listen(8000);
console.log(
  'Test server is running on \x1B[1m\x1B[32mhttp://localhost:8000/test/\x1B[39m, press Ctrl-C to stop.'
);

Can I have the full path please @odaysec ?

@gogo2464
Copy link

it sounds to be parser with

var uri = url.parse(req.url).pathname,
      filename;

I am going to read the parser and then I will tell you.

@gogo2464
Copy link

It is done! I tested! Exploit fully functional. Sorry for blaming @odaysec . exploit coming to publish in the next seconds.

@gogo2464
Copy link

Just copy and past this:

nc localhost 8000
GET /../../../../../etc/passwd HTTP/1.1
Host: localhost

The 2 next lines are importants :) The browser change the url itself... We have to use netcat.

@gogo2464
Copy link

I also have to ensure it is not my client side GET ../ ... that did everything.

@gogo2464
Copy link

Wonderfull! Just do echo -e "GET /../../../../etc/passwd HTTP/1.1\r\nHost: localhost\r\n\r\n" | nc localhost 8000 it worls perfectly 👍

@gogo2464
Copy link

Thank you very much to @odaysec for the reliable exploit.

@cure53 Thank you also to you for organizing the release (or advice for). You sound much more experienced than us. I did not found the self intro in the commits from @cure53 itself. Might be not deservable for a CVE but if he says he founds his own mistake, it is already a great source of interest for people who want to see how vulns appear in the wild.

I am not experienced in CVE publication. Does someone knows where is the proof in commit history that @odaysec has introduced the vuln please? I am also curious.

@gogo2464
Copy link

I am very confused with the commit where OdaySec has introduced the vulnerability.

I did:

$ git log --author=".*oday.*"
commit e9afd609397aa31b0747a766504f698fcb6ad0f7
Author: Zeroday BYTE <[email protected]>
Date:   Wed May 14 06:18:26 2025 +0700

    Update server.js

commit e242b4343b36e414db6118f20eccd574af7dd5ad
Author: Soheil Khodayari <[email protected]>
Date:   Tue Aug 16 20:05:33 2022 +0200

    Extra DOM Clobbering protection via SANITIZE_NAMED_PROPS config

And did not found no more.

@gogo2464
Copy link

@cure53 I am also very confused. Do you think he did not found?

@cure53
Copy link
Owner

cure53 commented Aug 16, 2025

@gogo2464 The "vulnerability" affects a test script that has no impact on any users or developers, please check the info available for more clarity.

@odaysec You have been permanently banned from the project and reported to Github for misinformation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants