build(dev-app): prevent directory traversal

[jelbourn]

No description provided.

[josephperrott]

LGTM

[devversion]

LGTM. Mind giving context on why this came up?

[devversion]

Would it be best to just disallow ../ in the requests? If we just want to go with that logic, then we could probably simplify it by doing it using path.relative? WDYT?

for (const rootPath of this._rootPaths) {
  const relativePath = path.relative(rootPath, path.join(rootPath, req.url));
  if (relativePath.startsWith('../')) { throw }
}

[jelbourn]

I used the approach recommended by Google's security team. As I see it, it's safer because it ensures that, at the very end, the resolved URL didn't escape the webserver root.

My first approach wouldn't have caught all cases (https://wikipedia.org/wiki/Directory_traversal_attack#Variations_of_directory_traversal), and is more future proof in case mre ways of directory traversal are introduced.

[devversion]

Okay, that sounds reasonable.

I'm unsure about path.resolve though. The root paths are not file system paths in the dev-server, but rather Bazel manifest/runfile paths, so resolving paths from the cwd is incorrect. It doesn't impact the URL escape check in general, but it's technically not correct. It might be better to use path.relative to determine if the paths escapes the root path.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}