Docs: limited path traversal bug in example code on how to avoid path traversal

[Felix Kaiser]

Hi,

this seems to be a docs issue only, I did a *very* short search and couldn't find any affected OpenStack code, and anyway it's a minor bug and I hope I'm not too pedantic, but a bug is a bug.

https://security.openstack.org/guidelines/dg_using-file-paths.html

In [1]: def is_safe_path(basedir, path, follow_symlinks=True):
   ...: # resolves symbolic links
   ...: if follow_symlinks:
   ...: return os.path.realpath(path).startswith(basedir)
   ...:
   ...: return os.path.abspath(path).startswith(basedir)
   ...:

In [2]: import os

In [3]: os.getcwd()
Out[3]: '/home/ad'

In [4]: is_safe_path("/home/ad", "../admin/.ssh/id_rsa")
Out[4]: True

[Tristan Cacqueray (tristan-cacqueray)]

That is odd, perhaps there is a paste error?

$ cat test.py
```
import os
def is_safe_path(basedir, path, follow_symlinks=True):
# resolves symbolic links
  if follow_symlinks:
    return os.path.realpath(path).startswith(basedir)
  return os.path.abspath(path).startswith(basedir)
print(is_safe_path("/home/ad", "../admin/.ssh/id_rsa"))
```
$ python test.py
False
$ python -V
Python 3.7.3

[Felix Kaiser (fxkr)]

Did you run this directly in a a subdir of /home/? It depends on the
current working dir.

fxkr@notebook ~ % md5sum test.py # see attached too - copy pasted from
your email
533afda1abb6c72995b4a3e125bf060d test.py
fxkr@notebook ~ % python test.py
True
fxkr@notebook ~ % python3 test.py
True
fxkr@notebook ~ % python -V
Python 2.7.16
fxkr@notebook ~ % python3 -V
Python 3.7.3
fxkr@notebook ~ % pwd
/home/fxkr
fxkr@notebook ~ % ls /home/ad /home/admin # This shouldn't matter much
(obviously if it exists shouldn't be a symlink)
ls: cannot access '/home/ad': No such file or directory
ls: cannot access '/home/admin': No such file or directory

And I mean just logically it seems clear there should be a bug, as it does
a string based prefix match in the end.

Felix

On Mon, Jul 22, 2019 at 1:05 PM Tristan Cacqueray <email address hidden>
wrote:

> That is odd, perhaps there is a paste error?
>
> $ cat test.py
> ```
> import os
> def is_safe_path(basedir, path, follow_symlinks=True):
> # resolves symbolic links
> if follow_symlinks:
> return os.path.realpath(path).startswith(basedir)
> return os.path.abspath(path).startswith(basedir)
> print(is_safe_path("/home/ad", "../admin/.ssh/id_rsa"))
> ```
> $ python test.py
> False
> $ python -V
> Python 3.7.3
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1815422
>
> Title:
> Docs: limited path traversal bug in example code on how to avoid path
> traversal
>
> Status in OpenStack Security Advisory:
> New
>
> Bug description:
> Hi,
>
> this seems to be a docs issue only, I did a *very* short search and
> couldn't find any affected OpenStack code, and anyway it's a minor bug
> and I hope I'm not too pedantic, but a bug is a bug.
>
> https://security.openstack.org/guidelines/dg_using-file-paths.html
>
> In [1]: def is_safe_path(basedir, path, follow_symlinks=True):
> ...: # resolves symbolic links
> ...: if follow_symlinks:
> ...: return os.path.realpath(path).startswith(basedir)
> ...:
> ...: return os.path.abspath(path).startswith(basedir)
> ...:
>
> In [2]: import os
>
> In [3]: os.getcwd()
> Out[3]: '/home/ad'
>
> In [4]: is_safe_path("/home/ad", "../admin/.ssh/id_rsa")
> Out[4]: True
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/ossa/+bug/1815422/+subscriptions
>

[Jeremy Stanley (fungi)]

It looks like in Python >=3.5 we can match against os.path.commonpath() as it will actually compare path elements for the common leading components, rather than just treating them as opaque strings.

[Jeremy Stanley (fungi)]

I've proposed this as a fix in https://review.opendev.org/771854 since it seemed like nobody else was interested in picking it up.

[Jeremy Stanley (fungi)]

That change merged a few days ago and was subsequently deployed to the site moments thereafter.