The client is supposed to see just a directory and its contents on the server (FS_ROOT).

And the server is supposed to convert the paths that it receives from the client to real paths that exist and do the file operations that the client requested on them:

I made these 2 functions to handle that and I want to ask if they are secure enough. I mean there should be no way for the client to fool the server to do something outside FS_ROOT

  function fromVirtualPath(virtPath){
    if(virtPath === '/' || virtPath === '.')
      return FS_ROOT;

    virtPath = virtPath.trim();

    if(virtPath[0] === '/')
      virtPath = virtPath.substr(1);

    const absPath = path.resolve(FS_ROOT, virtPath);

    if(absPath.indexOf(FS_ROOT) !== 0)
      throw new Error('Outside root dir - no permissions!');

    return absPath;
  }

  function toVirtualPath(absPath){
    return '/' + path.relative(FS_ROOT, absPath);
  }

Example real path: /www/site/public_html/yo

Client should see: /yo

The client is supposed to see just a directory and its contents on the server (FS_ROOT).

And the server is supposed to convert the paths that it receives from the client to real paths that exist and do the file operations that the client requested on them:

I made these 2 functions to handle that and I want to ask if they are secure enough. I mean there should be no way for the client to fool the server to do something outside FS_ROOT

  function fromVirtualPath(virtPath){
    if(virtPath === '/' || virtPath === '.')
      return FS_ROOT;

    virtPath = virtPath.trim();

    if(virtPath[0] === '/')
      virtPath = virtPath.substr(1);

    const absPath = path.resolve(FS_ROOT, virtPath);

    if(absPath.indexOf(FS_ROOT) !== 0)
      throw new Error('Outside root dir - no permissions!');

    return absPath;
  }

  function toVirtualPath(absPath){
    return '/' + path.relative(FS_ROOT, absPath);
  }

Example real path: /www/site./public_html/yo

Client should see: /yo

• javascript
• node.js
• security
• path

Share Improve this question Follow asked Aug 16, 2020 at 17:52 AlexAlex 66.2k185185 gold badges460460 silver badges651651 bronze badges

Add a ment  | 

6 Answers 6

Sorted by: Reset to default 2

About fromVirtualPath I would simply move the line virtPath = virtPath.trim(); to be the first line of the function, then it's ok.

If the values passed to toVirtualPath are always return values of fromVirtualPath, yes it is secure enough; other wise we could check if the value is a good absPath.

function fromVirtualPath(virtPath) {
  virtPath = virtPath.trim();

  if (virtPath === '/' || virtPath === '.')
    return FS_ROOT;

  if (virtPath[0] === '/')
    virtPath = virtPath.substr(1);

  const absPath = path.resolve(FS_ROOT, virtPath);

  if (absPath.indexOf(FS_ROOT) !== 0)
    throw new Error('Outside root dir - no permissions!');

  return absPath;
}

function toVirtualPath(absPath) {
  if (absPath.indexOf(FS_ROOT) !== 0)
    throw new Error('Bad absolute path!');

  return '/' + path.relative(FS_ROOT, absPath);
}

Your code is a bit insecure until you make use of the techniques provided by NODE.JS in the mentioned article. Try implementing the following code,

function fromVirtualPath(virtPath) {
  virtPath = virtPath.trim();

  if (virtPath === '/' || virtPath === '.')
    return FS_ROOT;

  if (virtPath.indexOf('\0') !== -1)
    throw new Error('That was evil.');

  const absPath = path.join(FS_ROOT, virtPath);

  if (absPath.indexOf(FS_ROOT) !== 0)
    throw new Error('Outside root dir - no permissions!');

  return absPath;
}

function toVirtualPath(absPath) {    
  return '/' + path.relative(FS_ROOT, absPath);
}

---

The following article from NODE.JS will be really helpful to you.

"How can I secure my code?"

Poison Null Bytes

Poison null bytes are a way to trick your code into seeing another filename than the one that will actually be opened.

if (filename.indexOf('\0') !== -1) {
  return respond('That was evil.');
}

Preventing Directory Traversal

This example assumes that you already checked the userSuppliedFilename variable as described in the "Poison Null Bytes" section above.

var rootDirectory = '/var/www/'; // this is your FS_ROOT

Make sure that you have a slash at the end of the allowed folders name

• you don't want people to be able to access /var/www-secret/, do you?.

  var path = require('path'); var filename = path.join(rootDirectory, userSuppliedFilename);

Now filename contains an absolute path and doesn't contain .. sequences anymore - path.join takes care of that. However, it might be something like /etc/passwd now, so you have to check whether it starts with the rootDirectory:

if (filename.indexOf(rootDirectory) !== 0) {
  return respond('trying to sneak out of the web root?');
}

Now the filename variable should contain the name of a file or directory that's inside the allowed directory (unless it doesn't exist).

Security is a plex matter. And you can never be sure.

Despite the fact that I couldn't find any flows in @RahulVerma answer I'll add my 2 cents...

The link that @RahulVerma posted is official but not a documentation per se. And in the documentation there is nothing about Poison Null Bytes ...strange isn't it.

And that makes you think: maybe, just maybe, when the fs and/or path modules were written authors didn't put enough effort into security considerations, or just missed that. Yes, maybe there are some good reasons for you and not the fs/path to handle the \0. But also wouldn't it be better if everyone was protected from \0 by default? And only for some rear occasions you could explicitly set an option to allow \0 in paths.

So... what am I trying to say is: security is hard even for the best of us, and without proper peer review (currently, less than 100 views on this question do not strike me as a "proper peer review") or, better yet, a history of successful time in production, you should not be satisfied with these answers (my included) saying "It's OK, if you add this or that".

Why don't you use some code that already was tested in battles instead of trying to write a secure code by yourself?

E.g serve-static is used in Express.
^{(Probably it doesn't meet your needs - it's static after all, but you get the idea)}

Even if you don't want another dependency in your project you can at least study and copy from the implementation that proved itself. (But, yes, it doesn't seem different from the @RahulVerma answer)

That said. I'd like to point out that:

1. If you'd copy the implementation, you can make a mistake while doing so.

2. Even if your code is safe, consider how safe do you manage your code. Will it be safe tomorrow?

3. Even well tested libraries and engines can, and often do, have bugs, and fall prey to 0day exploits

   Oh! Just found: https://github./autovance/ftp-srv/issues/167 It's about the library that was suggested in another question of yours.

So, if you decide (or if you'll be assured) that now you code is surely safe don't stop on that! Add an extra layer of security to it anyway:

restrict the server's access to folders outside of the /www/site./public_html/ on an OS level.

The following principles can be applied to secure client access to paths relative to the web root:

1. Restrict access outside of your public web root folder to your service. Rationale: begin with ZERO trust.
2. Split the path provided by the user into parts. This will remove leading '/' and all '/' separators leaving only the parts of the path. Better yet, use whitelisting for path parts to restrict acceptable characters in a path part using a regular expression. Rationale: sanitize user input
3. Validate each part sequentially for existence assuming that the first part starts from the web root as it is intended. Disallow .. (parent dir) in part names (to prevent traversal outside the web root folder). Rationale: sanitize user input and validate user input
4. Avoid using symbolic links under the web root folder (to prevent traversal outside the web root folder). Rationale: reduce attack surface
5. Fail early with an error upon encountering the first invalid part. Rationale: reduce attack surface

To optimize system calls, you can do the check for .. and part whitelisting in one pass. If there are any .. in the path or offending parts, return an error. Otherwise, split the parts and rebuild the absolute path string by concatenating them with your web root and do one existence check instead of multiple folder existence checks along the path.

Instead of trying to validate every path yourself, let the operating system do it for you! This is a good example of an application that could use a chroot.

Here is an example of an npm library which creates a chroot.

> var chroot = require("chroot")
> var fs = require("fs")
> chroot('/virtual/root/here', 'nobody')
> fs.readdir(".", function(err, files) { console.log(files); }) // Lists virtual root
> fs.readdir("..", function(err, files) { console.log(files); }) // Also lists virtual root
> fs.readdir("/", function(err, files) { console.log(files); }) // ALSO lists virtual root

Should you run this script as root, it immediately changes the user to "nobody" and sandboxes you to your virtual root. This prevents the script from accessing anything outside it, and the program can't chroot out either, as it's no longer running as root.

Now that you are chrooted into your virtual root, using "/" will give you a directory listing of your virtual root - essentially, you can use your virtual path directly in fs.readdir()!

Need to access some specific files outside the new root? Use microservices! You can run a node.js instance in the background as your file accessor, and municate between your main server and your file accessor. Having two nodejs instances not only allows your background task to sandbox itself, but also allows you to make use of multithreading.

Yours is a basic java code. In real time scenarios, these basic java code should not be deployed on server side and we can't expect secuirty out of this.

To add the security check to this java code, many APIs e as part of Spring framework but since we are writing java code then we can make use of java NIO package only, API name WatchService and WatchEvent

class DirectoryWatchTest {
public static void main(String[] args) {
    try {
        WatchService watchService = FileSystems.getDefault().newWatchService();
        Path path = Paths.get("C:/");

        /**
         * The register() method of the Path class takes a WatchService object and an event type for which the 
         * application needs to get notified. 
         * 
         * The supported event types are: 
         * ENTRY_CREATE: indicates if a directory or file is created. 
         * ENTRY_DELETE: indicates if a directory or file is deleted. 
         * ENTRY_MODIFY: indicates if a directory or file is modified. 
         * OVERFLOW: indicates if the event might have been lost or discarded. This event is always implicitly 
         * registered so we don't need to explicitly specify it in the register() method. */
        path.register(watchService, ENTRY_CREATE, ENTRY_DELETE, ENTRY_MODIFY);
        while (true) {
            WatchKey key;
            try {
                key = watchService.take();
            } catch (InterruptedException ex) {
                return;
            }
            
        /** 
         * The whole work flow:
         * A Watchable object is registered with a watch service by invoking its register method, 
         * returning a WatchKey to represent the registration. 
         * 
         * When an event for an object is detected, the key is signalled, and if not currently signalled, 
         * it is queued to the watch service so that it can be retrieved by consumers that invoke the poll or 
         * take methods to retrieve keys and process events. 
         * 
         * pollEvents List<WatchEvent<?>> pollEvents() method retrieves and removes all pending events for 
         * this watch key, returning a List of the events that were retrieved. Note that this method does not 
         * wait if there are no events pending. */
        
        for (WatchEvent<?> event : key.pollEvents()) {
            WatchEvent.Kind<?> kind = event.kind();
            @SuppressWarnings("unchecked")
            WatchEvent<Path> ev = (WatchEvent<Path>) event;
            Path fileName = ev.context();
            System.out.println(kind.name() + ": " + fileName);
            
            if (kind == ENTRY_MODIFY && fileName.toString().equals("DirectoryWatchTest.java")) {
                    System.out.println("My source file has changed!!!");
                    System.out.println("My source file has changed!!! - Modified");
                }
            }
        /**Once the events have been processed the consumer invokes the key's reset method to reset the 
         * key which allows the key to be signalled and re-queued with further events.*/
        boolean valid = key.reset();
            if (!valid) {
                break;
            }
        }
    } catch (IOException ex) {
        System.err.println(ex);
    }
}

}

This kind of basic security check can be put in java code. The user will be able to watch the url unless and until we don't get hold of protocol and hide it via @PutMapping or implementing security based API's in this but for that we need framework based API's

enter code here