PosixPath() constructor should not accept strings with embedded NUL bytes

[ischwabacher]

BPO	22147
Nosy	@pitrou, @vajrasky, @iritkatriel
Files	embedded_null_in_path.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2014-08-05.19:44:08.428>
labels = ['type-bug', 'library', '3.11']
title = 'PosixPath() constructor should not accept strings with embedded NUL bytes'
updated_at = <Date 2021-09-10.22:26:42.024>
user = 'https://bugs.python.org/ischwabacher'

bugs.python.org fields:

activity = <Date 2021-09-10.22:26:42.024>
actor = 'iritkatriel'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2014-08-05.19:44:08.428>
creator = 'ischwabacher'
dependencies = []
files = ['36327']
hgrepos = []
issue_num = 22147
keywords = ['patch']
message_count = 5.0
messages = ['224880', '224882', '225095', '225102', '401616']
nosy_count = 4.0
nosy_names = ['pitrou', 'vajrasky', 'ischwabacher', 'iritkatriel']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue22147'
versions = ['Python 3.11']

[ischwabacher]

This is listed as a python3.4 issue even though I only tried this on the python2.7 backport because I don't have a python3 handy, but I was not able to find an indication, either here or elsewhere, that this had been addressed. Please forgive me if it has.

The pathlib.PosixPath() constructor currently accepts strings containing NUL bytes, converting them into paths containing NUL bytes. POSIX specifies that a pathname may not contain embedded NULs.

It appears that PosixPath.stat() is checking for embedded NUL, but PosixPath.open() is not! For safety, constructing a PosixPath with embedded NULs should be forbidden.

pathlib.WindowsPath() should probably receive the same treatment.

Observed behavior:

>>> from pathlib import Path

>>> Path("\0I'm not malicious, I'm mischievous!")
PosixPath("\x00I'm not malicious, I'm mischievous!")

>>> _.open()
Traceback (most recent call last):
 File "<stdin>", line 1, in <module>
 File ".../site-packages/pathlib.py", line 1077, in open
 return io.open(str(self), mode, buffering, encoding, errors, newline)
IOError: [Errno 2] No such file or directory: ''

>>> Path('/') / _
PosixPath("/\x00I'm not malicious, I'm mischievous!")

>>> _.open()
Traceback (most recent call last):
 File "<stdin>", line 1, in <module>
 File ".../site-packages/pathlib.py", line 1077, in open
 return io.open(str(self), mode, buffering, encoding, errors, newline)
IOError: [Errno 21] Is a directory: "/\x00I'm not malicious, I'm mischievous!"

>>> _.stat()
Traceback (most recent call last):
 File "<stdin>", line 1, in <module>
 File ".../site-packages/pathlib.py", line 1051, in stat
 return self._accessor.stat(self)
 File ".../site-packages/pathlib.py", line 346, in wrapped
 return strfunc(str(pathobj), *args)
TypeError: must be encoded string without NULL bytes, not str

>>> p1 = Path('/etc/passwd\0/hello.txt').open()

>>> p2 = Path('/etc/passwd').open()

>>> os.path.sameopenfile(p1.fileno(), p2.fileno())
True  # DANGER WILL ROBINSON!

Expected behavior:

>>> Path("/\0I'm not malicious, I'm mischievous!")
...
ValueError: Illegal byte '\x00' in path

[ischwabacher]

Further digging reveals that the issue with open() was fixed in bpo-13848 (the bug was in the io module). I still believe that this should fail in the pathlib.Path constructor, but this is less of a security issue.

[pitrou]

This sounds like a reasonable request indeed.

[vajrasky]

Here is the patch.

[iritkatriel]

If we do this then there are tests tha break, for instance test_is_symlink has this:

self.assertIs((P / 'fileA\x00').is_file(), False)

[barneygale]

-1 from me, as it opens the door to validating filenames on Windows too, which is a can of worms. It will also slow path construction. pathlib.Path() currently accepts any string, and I think that's reasonable.

[pitrou]

@barneygale I think you have a good point here.