os.path.normpath of relative path r".\C:\x" returns absolute path r"C:\x" on Windows, similar in pathlib

[gpshead]

os.path.normpath normalizes "./" or "../" path elements to clean the path.

As os.path.normpath doesn't consider a case where the normalized path starts with a drive letter, a relative path with a drive letter-like path element will be normalized to an absolute path. This behavior can result in a path traversal, depending on the implementation.

The minimal snippet to reproduce this behavior is the following:

> os.path.normpath("./C:/Windows/System32")

This snippet will return "C:\Windows\System32", which is an absolute path on Windows. (tested with Python 3.11.1 on Windows)

This vulnerability is similar to CVE-2022-29804 of Go.

as reported to security@ by RyotaK on 2022-12-09.

---

Golang solved their issue in golang/go#52476 which may be helpful to look at.

The Python Security Response Team agreed that this issue did not require an embargo.

It looks like pathlib probably also has issues in this area:

>>> p1 = pathlib.PureWindowsPath('P:\\windows')
>>> p2 = pathlib.PureWindowsPath('.\\P:\\windows')
>>> p1 == p2
False
>>> p1
PureWindowsPath('P:/windows')
>>> p2
PureWindowsPath('P:/windows')
>>> p1.is_absolute()
True
>>> p2.is_absolute()
False
>>> str(p1) == str(p2)
True

[eryksun]

I wouldn't hold up ".\C:\Windows\System32" as a shining example because it's an invalid path. Removing the leading "." component in this case doesn't break a path that would otherwise work. That said, removing the leading "." component leads to a couple of other issues that have been discussed previously.

The first is the need to use a leading "." component in order to avoid ambiguity with a single-letter filename that contains named file streams. For example, ".\C:spam" refers to a data stream named "spam" in a file or directory named "C" in the current working directory. Without the leading "." component, "C:spam" is a file or directory named "spam" relative to the current working directory of drive "C:". (Each drive has its own working directory, which defaults to the root directory.)

The other issue is that removing the leading "." component can change the meaning of a path of an executable that's passed to POSIX exec*() or WinAPI CreateProcess*W() or SearchPathW(). In a search context, the path "./spam" is limited to the current directory, while just "spam" is searched for. (WinAPI CreateProcess*W() and SearchPathW() also search for "spam\eggs", but ".\spam\eggs" is limited to the current directory.)

IIRC, neither of these issues has been seen as urgent enough to change the long-standing behavior normpath().

[eryksun]

It's worth noting that WinAPI PathCchCanonicalizeEx() has the same behavior for the garbage in, garbage out case of r".\C:\x". Also, PathCchCombineEx() canonicalizes the result the same way. For example:

>>> import ctypes
>>> winpath = ctypes.OleDLL('api-ms-win-core-path-l1-1-0')
>>> out_path = (ctypes.c_wchar * 1000)()

>>> winpath.PathCchCanonicalizeEx(out_path, 1000, r'.\C:\spam', 0)
0
>>> out_path.value
'C:\spam'

>>> winpath.PathCchCombineEx(out_path, 1000, r'.\C:\spam', 'eggs', 0)
0
>>> out_path.value
'C:\\spam\\eggs'

On Windows, the internal functions _Py_join_relfile() and _Py_add_relfile() call PathCchCombineEx(). _Py_join_relfile() isn't used on Windows. _Py_add_relfile() is used by getpath_joinpath() in "Modules/getpath.c". This is the joinpath() function in "Modules/getpath.py". However, I don't think it's used on Windows in a way that's vulnerable. Unless I missed a case, the first item is always a qualified path such as the prefix directory or the directory of the executable or interpreter DLL.

[zooba]

Agreed, I don't think Python itself is vulnerable here, but our users may have written code that is exploitable by an error case becoming a legitimate case:

path = input("Provide your path:")
# path = r".\D:\secrets.txt"
assert not os.path.isabsolute(path)
path = os.path.normpath(path)
print("Reading from", path)
...

First immediate fix should be to update the docs to discourage calling normpath on relative paths if you're going to use them later (it's fine enough for display purposes). You should always join to an absolute path first. We jump through as many hoops as we can to make it work, but it's still going to be less reliable than not doing it. (And if you're really concerned about symlinks or junctions, you shouldn't normpath at all, though you're going to run into inconsistencies all over in that case anyway.)

I'm inclined to think that the best fix is for normpath to preserve one leading ./ segment on all platforms (unless we have a leading ../, or an absolute path). Right now, it explicitly skips that one segment and drops it from the result, so really we're just going to bring it back at the end of the algorithm unless we start with ../.

I can't come up with a reason to not do this on all platforms, especially given it's common code for all platforms, but maybe there's something I missed?

[eryksun]

I'm inclined to think that the best fix is for normpath to preserve one leading ./ segment on all platforms

We would also have to handle cases such as "spam/../C:eggs", which is a valid path for a file named "C" containing a data stream named "eggs" . This example should be normalized as r".\C:eggs".

I suggest the following change to the final step of the pure Python implementation:

    # Prepend curdir either for an empty path or to avoid changing the
    # meaning of a path (e.g. avoid "spam/../C:eggs" -> "C:eggs").
    if not prefix and (not comps or comps[0][1:2] == colon):
        comps.insert(0, curdir)
    return prefix + sep.join(comps)

This is a minimal change, which doesn't always preserve an initial "." component in the path argument. We could easily implement the latter as well by checking the initial value of comps[0] before processing the path components.

[zooba]

Good point. Perhaps this check may also work?

new_path = old_normpath(path)
try:
    i = new_path.index(':')
    if i < path.index(':'):
        # colon moved closer to the start
        try:
            new_path.index('\\', 0, i) # find slash before colon
        except ValueError:
            new_path = ".\\" + new_path # colon in first segment
except ValueError:
    pass # no colon

Should be easy to tack onto the end of either approach, and will avoid changing anything if the path didn't change. There shouldn't be any scenarios where the first colon moves later in the path, and anywhere it stays at the same location we can ignore.