Using ".." allows loading elements from outside of `element-path`

[abderrahim]

If I have a project with the conventional element-path: elements, I can write an element in the root directory of the project (say test.bst) and bst will happily load it if I do bst show ../test.bst or bst build ../test.bst.

[aidanmct]

You can also create a symlink pointing anywhere on the filesystem and builstream will load it, eg. elements/foo.bst -> /opt/bar.bst.

[nathanwilliams-ct]

It might be worth switching Buildstream to using PathLib instead of handling raw strings for all paths in buildstream, as it will make it much easier to handle the paths and more compatible with Buildstreams OOP model?

_basedir = Path('elements')
target = '../../bananas/my_element.bst'

element_path = (_basedir / target).resolve()
try:
  element_path.relative_to(_basedir.resolve())  # or use `is_relative_to` in an if statement
except ValueError as e:
   # Handle the directory traversal Error

old way, with os path:

_basedir = 'elements'
target = '../../bananas/my_element.bst'
element_path = os.path.join(_basedir, target)

actual_basedir = os.path.realpath(_basedir)
actual_element_path = os.path.realpath(element_path)

if os.path.commonprefix(actual_element_path,actual_basedir) != actual_basedir: 
    # Handle the directory traversal Error

both approaches resolve symlinks and verify you are in the folder you intended to be in, a classic example of a 'directory traversal attack'. Although nothing is at risk here, so hardly an attack.

[abderrahim]

What I would suggest here is to disallow . and .. in path names, and not to dereference symlinks when loading the files.

Ideally, I would move this to use the Directory API, which already has some protections around this (even if they might not be enough).

[nathanwilliams-ct]

What I would suggest here is to disallow . and .. in path names, and not to dereference symlinks when loading the files.

Ideally, I would move this to use the Directory API, which already has some protections around this (even if they might not be enough).

I don't think preventing . and .. explicitly is the way to go here, as unlikely as it is, what happens if other path shortcuts are introduced? You again end up with this problem. (e.g. zsh supports '...' to go up two levels).

We should use the standard library functions/methods to handle this, so it will get maintained properly by the upstream python team, instead of custom implementations that we need to maintain. If the implementations are too slow, lets contribute an optimised implementation to upstream python (as hard as that may be), or ditch python.

[aidanmct]

I don't think preventing . and .. explicitly is the way to go here, as unlikely as it is, what happens if other path shortcuts are introduced? You again end up with this problem. (e.g. zsh supports '...' to go up two levels).

That's massively overthinking it; path resolution is handled by the kernel, not the shell. And even if I don't like it, ... is a valid filename.

Personally I don't see anything wrong with relative paths or symlinks in projects. Even if they are pointless, why add code to forbid them if it isn't causing any harm?

The only question for me is junctions... If they aren't sandboxed and can escape their project by pointing to arbitrary files on the host filesystem using relative paths or symlinks then that is fairly bad, given bst can download projects from the internet automatically without verification (a git repo, one of the most common ways to use junctions, provides no defense against tampering). And this goes for all ways that a project can direct bst to access files.