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] in assetModuleFilename may unexpectedly traverse parent directories #11937

Closed
andersk opened this issue Nov 6, 2020 · 8 comments
Closed

Comments

@andersk
Copy link

andersk commented Nov 6, 2020

Bug report

What is the current behavior?

With output.assetModuleFilename configured as "[path][name][ext]", referencing an asset from outside the project root via .. or a symlink through .. causes a file to be written outside of the output.path directory.

If the current behavior is a bug, please provide the steps to reproduce.

$ echo Hello >../../foo.txt
$ cat >webpack.config.js <<EOF
module.exports = {
  entry: "../../foo.txt",
  output: { assetModuleFilename: "[path][name][ext]" },
  module: {
    rules: [{ test: /\.txt$/, type: "asset/resource" }],
  },
};
EOF
$ npm i webpack webpack-cli

+ [email protected]
+ [email protected]

$ cat ../foo.txt
cat: ../foo.txt: No such file or directory
$ npx webpack
[webpack-cli] Compilation finished
asset main.js 711 bytes [emitted] [minimized] (name: main)
asset ../../foo.txt 6 bytes [emitted] [from: ../../foo.txt] (auxiliary name: main)
runtime modules 1.06 KiB 2 modules
../../foo.txt 42 bytes (javascript) 6 bytes (asset) [built] [code generated]
webpack 5.4.0 compiled successfully in 171 ms
$ cat ../foo.txt
Hello

webpack has created dist/../../foo.txt, i.e. ../foo.txt.

What is the expected behavior?

Given a similar configuration using loader: "file-loader", options: { name: "[path][name].[ext]" }, webpack instead creates dist/_/_/foo.txt, which seems like a reasonable way to avoid the unexpected directory traversal.

Other relevant information:
webpack version: 5.4.0
Node.js version: 14.15.0
Operating System: Linux
Additional tools:

@webpack-bot
Copy link
Contributor

For maintainers only:

  • webpack-4
  • webpack-5
  • bug
  • critical-bug
  • enhancement
  • documentation
  • performance
  • dependencies
  • question

@alexander-akait
Copy link
Member

alexander-akait commented Nov 6, 2020

Sorry, expected, file-loader uses loader-utils (will be deprecated in the near future) for creating filename, but it is buggy, there are a lot of other cases when it is valid

@andersk
Copy link
Author

andersk commented Nov 6, 2020

A directory traversal bug is expected?

To be clear, I don’t care about reproducing the exact behavior of file-loader—I’m just saying webpack shouldn’t pick a path that’s outside the directory I told it to output.

@alexander-akait
Copy link
Member

alexander-akait commented Nov 6, 2020

Where you see bug? We just write file above output.path as you declare

@alexander-akait
Copy link
Member

When you use fs method and using ../../ what you expected? Do not do magic here

@andersk
Copy link
Author

andersk commented Nov 11, 2020

What I expect is for webpack to convert the .. into _ (like file-loader does) or something else that doesn’t traverse parent directories. You might call this “magic”; I would call it a minimal sanitation step to avoid unexpectedly writing outside the output directory or outside the project root entirely.

If it helps, this affects Zulip even without any explicit .. references because

  • our node_modules is a symlink to a directory outside the project root,
  • we need [path] as a disambiguator because we reference two different files within node_modules with the same [name][ext],
  • we don’t want to use [hash] in dev mode because it interacts poorly with webpack-dev-server.

This would also affect a project where node_modules is a normal directory that’s simply at a higher level than webpack.config.js, e.g.

./package.json

{
  "dependencies": {
    "webpack": "^5.4.0",
    "webpack-cli": "^4.2.0"
  }
}

./projects/website/webpack.config.js

module.exports = {
  entry: "webpack/README.md",
  output: { assetModuleFilename: "[path][name][ext]", publicPath: "" },
  module: {
    rules: [{ test: /\.md$/, type: "asset/resource" }],
  },
};

@andersk
Copy link
Author

andersk commented Jun 7, 2023

For the logs: this was re-reported by someone else as

@alexander-akait
Copy link
Member

@andersk Yeah, we want to reconsider it, make sense to implement something new to solve it

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

3 participants