Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Path Manipulation in writeFile #2144

Open
dbohannon opened this issue May 23, 2018 · 3 comments
Open

Path Manipulation in writeFile #2144

dbohannon opened this issue May 23, 2018 · 3 comments

Comments

@dbohannon
Copy link

The filename argument passed to the writeFile() function in file-util.js may be manipulated to write arbitrary directories on the filesystem. The writeFile() function attempts to sanitize the untrusted filename, but calls the mkdirRecursive() function on the untrusted filename before the filename is sanitized (https://github.com/google/traceur-compiler/blob/master/src/node/file-util.js#L56)

For example, passing the filename "../test/target.txt" on a *nix system will create the directory "test" in the parent directory. An attacker can use the dot-dot-slash path manipulation technique to create directories anywhere on the filesystem.

function writeFile(filename, contents) {
  // Compute the output path
  var outputdir = fs.realpathSync(process.cwd());
  mkdirRecursive(path.dirname(filename));
  var filedir = fs.realpathSync(path.dirname(filename));
  filedir = removeCommonPrefix(outputdir, filedir);
  outputdir = path.join(outputdir, filedir);

  mkdirRecursive(outputdir);
  var outputfile = path.join(outputdir, path.basename(filename));
  fs.writeFileSync(outputfile, contents, 'utf8');
}
@arv
Copy link
Collaborator

arv commented May 23, 2018

Not sure what you are suggesting here?

I think it is important to allow paths starting with .. (or / for that matter). If you are accepting paths from an untrusted source you might want to consider checking the path before handing it to Traceur.

@dbohannon
Copy link
Author

Maybe I should clarify. If I submit the filename "../test/target.txt" and my current directory is "/tmp/traceur", the file is written to "/tmp/traceur/test/target.txt". However, the writeFile() function ALSO creates the empty directory "/tmp/test". This appears to be a bug that allows the user to write this "duplicate" directory structure anywhere on the filesystem, when the function is attempting to only write to the current directory. Regardless, this "duplicate" directory is unintended, is it not?

@arv
Copy link
Collaborator

arv commented May 23, 2018

I see. Yeah. That seems like a bug. The intent is to create the directory where the output files/dir are going.

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

No branches or pull requests

2 participants