Restrict path access to prevent path traversal in secure coding example

[Ron Ritchey]

This bug tracker is for errors with the documentation, use the following as a template and remove or add fields as you see fit. Convert [ ] into [x] to check boxes:

- [X] This doc is inaccurate in this way: ______
- [ ] This is a doc addition request.
- [X] I have a fix to the document that I can paste below including example: input and output.

If you have a troubleshooting or support issue, use the following resources:

 - The mailing list: https://lists.openstack.org
 - IRC: 'openstack' channel on Freenode

-----------------------------------
Release: 0.0.1.dev228 on 2021-01-21 16:53:50
SHA: d4785ae6fdb8b9fd87748b508c10622f6c0ae027
Source: https://opendev.org/openstack/ossa/src/doc/source/guidelines/dg_using-file-paths.rst
URL: https://security.openstack.org/guidelines/dg_using-file-paths.html

is_safe_path function throws TypeError: expected str, bytes or os.PathLike object, not bool

It likely should either be written as...

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

or...

def is_safe_path(basedir, path, follow_symlinks=True):
    # resolves symbolic links
    if follow_symlinks:
         matchpath = os.path.realpath(path)
    else:
         matchpath = os.path.abspath(path)
    return basedir == os.path.commonpath((basedir, matchpath))

[Jeremy Stanley (fungi)]

Thanks, you're right that example is definitely broken. I rewrote it in an attempt to address bug 1815422, but for some reason I decided to return something other than a bool in the rewrite. I'll work on an alternative to get it back to what the goal of the original example was.

[Jeremy Stanley (fungi)]

Oh, I see the problem, it is still correctly trying to return a bool from that function, I just failed to strip off the old startswith() methods for the assignments leading up to it in the previous fix.

[Jeremy Stanley (fungi)]

Yep, the fix I landed on is the same as your second example. I think that's what I originally meant to have there just didn't completely clean up the prior version of the example.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/ossa/+/799282

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/ossa/+/799282
Committed: https://opendev.org/openstack/ossa/commit/b05ba124452ffe17f3f7223b740e977ca5ee0b01
Submitter: "Zuul (22348)"
Branch: master

commit b05ba124452ffe17f3f7223b740e977ca5ee0b01
Author: Jeremy Stanley <email address hidden>
Date: Fri Jul 2 17:12:59 2021 +0000

    Correct is_safe_path example in guidelines

    A previous rework of the directory traversal mitigation example in
    I3f8d3760daceb9e62396ae21b0d915ae07eff303 was not correctly cleaned
    up, and left some unintended startswith method invocations behind.
    Get rid of those, and also correct a wrong parameter name in the
    main function while we're at it, as well as fixing some incorrect
    indentation.

    Change-Id: Ie5347f3b6cc8e689440db0aaf552d52ad37c231c
    Closes-Bug: #1928544