added multiuser support and associated tests by jahrules · Pull Request #192 · tox-dev/filelock

jahrules · 2023-02-02T16:52:28Z

new option FileLock(multiuser = True) (defaults to False):

results in Lock File being created with rw-rw-rw- permissions so that all users on the system can use the lockfile.

jahrules · 2023-02-20T18:25:45Z

flake8 doesnt like the import filemode in test_filelock.py that I added to check the filemode with this addition. Aside from that; all tests should pass now.

Apologies for the gap from the original PR to these fixes. It was some time before I could access a windows box to debug.

jahrules · 2023-02-26T01:01:54Z

I wanted to post perhaps a few clarifications.

This PR addresses requests noted in the following issues:
#112
#70

As mentioned in issue #112 I updated the variable name from mode to flags - which is a finding that I made independently before finding it mentioned in the aforementioned issue.

gaborbernat

Add changelog entry here https://github.com/tox-dev/py-filelock/blob/main/docs/changelog.rst

gaborbernat · 2023-03-13T16:57:46Z

+        self,
+        lock_file: str | os.PathLike[Any],
+        timeout: float = -1,
+        multi_user: bool = False,


I'm not sure about this interface. Should we just expose access_control instead and allow them to set it to whatever they want?

I'm open to this idea. My only concern is that when setting permissions for "group" or "all" you also have to set os.umask(0) so a change to that interface will also require extra checking for the requested permissions to set the umask if necessary

I'm alright to make that check 👍

jahrules · 2023-03-13T20:41:06Z

I have some additional information on this. The default to mode in os.open is 511 (0o777 in octal). So the r/w bits are set by default. The only reason that the default mode doesn't make the files globally r/w is the lack of the os.umask call. So, even with exposing the mode directly, I believe we need a way for users to specify if they intend the lock to be multiuser or not. Maybe a solution would to both allow the user to specify their own mode and specify if multi-user is intended but this may lead to more confusion. Open to thoughts on this.

…

On Mon, Mar 13, 2023, 3:48 PM Bernát Gábor ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/filelock/_api.py <#192 (comment)>: > @@ -39,14 +39,20 @@ def __exit__( class BaseFileLock(ABC, contextlib.ContextDecorator): """Abstract base class for a file lock object.""" - def __init__(self, lock_file: str | os.PathLike[Any], timeout: float = -1) -> None: + def __init__( + self, + lock_file: str | os.PathLike[Any], + timeout: float = -1, + multi_user: bool = False, I'm alright to make that check 👍 — Reply to this email directly, view it on GitHub <#192 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APCURVAHRHBMU3UFCN3PX2DW352XVANCNFSM6AAAAAAUPIPPVM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

jahrules · 2023-03-13T20:43:02Z

Another potential solution would be to make the filelock default mode 0o700 but I'm not comfortable making the call on if we should deviate from the os open default or not.

…

On Mon, Mar 13, 2023, 4:40 PM Greg Taeger ***@***.***> wrote: I have some additional information on this. The default to mode in os.open is 511 (0o777 in octal). So the r/w bits are set by default. The only reason that the default mode doesn't make the files globally r/w is the lack of the os.umask call. So, even with exposing the mode directly, I believe we need a way for users to specify if they intend the lock to be multiuser or not. Maybe a solution would to both allow the user to specify their own mode and specify if multi-user is intended but this may lead to more confusion. Open to thoughts on this. On Mon, Mar 13, 2023, 3:48 PM Bernát Gábor ***@***.***> wrote: > ***@***.**** commented on this pull request. > ------------------------------ > > In src/filelock/_api.py > <#192 (comment)>: > > > @@ -39,14 +39,20 @@ def __exit__( > class BaseFileLock(ABC, contextlib.ContextDecorator): > """Abstract base class for a file lock object.""" > > - def __init__(self, lock_file: str | os.PathLike[Any], timeout: float = -1) -> None: > + def __init__( > + self, > + lock_file: str | os.PathLike[Any], > + timeout: float = -1, > + multi_user: bool = False, > > I'm alright to make that check 👍 > > — > Reply to this email directly, view it on GitHub > <#192 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/APCURVAHRHBMU3UFCN3PX2DW352XVANCNFSM6AAAAAAUPIPPVM> > . > You are receiving this because you authored the thread.Message ID: > ***@***.***> >

gaborbernat · 2023-03-14T01:46:35Z

I believe we need a way for users to specify if they intend the lock to be multiuser or not.

But there's no such thing as multi user. Is just mangles to acces to the lock created. It just happens that a given combination of flags enable multiple users to use the same lock file.

jahrules · 2023-03-14T03:07:32Z

Well yes but it's the combination of flags plus umask. Typical default for umask (at least on unix based OSes) is 0o133 to deny write to group/all. So even though the os.open has a default mode of 0o777; the umask default denies write making it effectively 0o644 which is why "other users" can't use the same lock. So to distill this to the core; I want to ask filelock users "do you want other users to use this lock?" and modify umask to permit this. Modifying mode and umask is pretty advanced for most users which is why Python put that implementation into os.open and didn't expose it in the standard "open". Maybe the best option is to leave mode alone as I think it highly unlikely that users would need granular permissions. The three options I can reasonably see people wanting are "self" locks (as they are today), "group" locks (where the "group" bit is set to write (umask 0o113), and "global" locks (umask 0o111). The use case for the 3rd would be like if an application user and human user both needed to use the lock. The difference between 2 and 3 is, however, minimal.

…

On Mon, Mar 13, 2023, 9:46 PM Bernát Gábor ***@***.***> wrote: I believe we need a way for users to specify if they intend the lock to be multiuser or not. But there's no such thing as multi user. Is just mangles to acces to the lock created. It just happens that a given combination of flags enable multiple users to use the same lock file. — Reply to this email directly, view it on GitHub <#192 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APCURVBZBFTZCMHPF2W4BILW37EYPANCNFSM6AAAAAAUPIPPVM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

gaborbernat · 2023-03-14T14:36:15Z

-            fd = os.open(self._lock_file, open_mode)
+            open_flags = os.O_RDWR | os.O_CREAT | os.O_TRUNC
+            if self._multi_user is True:
+                os.umask(0)


Shouldn't this be in a try/finally, so after creation we restore the previous value? Also do we need this conditionality? Can't we just always set umask to 0 before creation and restore after? Technically if the umask to set to something non zero, the open mode 511 will not be respected. Also this umask thing is repeated across files so any reason can't be a contextmanager in base?

agree completely. I would suggest that by going that route, the default should be 0o644 so that the 'net effect' of this change is that the default behavior remains the same. Let me know if you would prefer something else.

Also, I think that I will set the umask to 0o111 instead of 0; since there is no reason that the lock files should be executable.

Changes in progress.

gaborbernat · 2023-03-15T18:25:34Z

            raise
+        finally:
+            # reset umask to initial value
+            os.umask(previous_umask)


move comment into the finally line

gaborbernat · 2023-03-15T18:26:26Z

        :param timeout: default timeout when acquiring the lock, in seconds. It will be used as fallback value in
        the acquire method, if no timeout value (``None``) is given. If you want to disable the timeout, set it
        to a negative value. A timeout of 0 means, that there is exactly one attempt to acquire the file lock.
+        : param multiuser: if True, make the lock file permissions rw/rw/rw for all users


mode not multiuser now 👍

gaborbernat · 2023-03-15T18:27:59Z

@@ -1,5 +1,8 @@
 Changelog
 =========
+v3.9.1 (2023-03-13)


3.10.0 as is feature

gaborbernat · 2023-03-15T19:26:28Z

        the acquire method, if no timeout value (``None``) is given. If you want to disable the timeout, set it
        to a negative value. A timeout of 0 means, that there is exactly one attempt to acquire the file lock.
-        : param multiuser: if True, make the lock file permissions rw/rw/rw for all users
+        : param mode: file permissions for the lockfile to have. default: 0o644 (-rw-r--r--)


remove default, it's already in the header of the method

gaborbernat · 2023-03-15T19:27:58Z

        start_time = time.monotonic()
+        previous_umask = os.umask(0)


Shouldn't this try/catch be scopped just to _acquire?

Looking at _acquire; it will never throw a BaseException error, so it should be safe to just wrap self._acquire() with a set and reset.

The try block is probably a candidate for future improvement because it's quite strangely written; but that seems outside of the scope of this PR.

What if you get a keyboardinterrupt; let's use try/finally please.

gaborbernat

Can you rebase on main, and fix merge conflict?

for more information, see https://pre-commit.ci

jahrules · 2023-03-15T20:23:30Z

rebased and fixed all conflicts.

This update addresses the following vulnerabilities: - filelock on Windows ignores the `mode` argument. `FileLock(path, mode=0o600)` creates the same file as `FileLock(path)`: one with no security descriptor of its own (the Windows record that says who may access a file). Its access list, or ACL, is whatever the lock directory hands down to new files. filelock creates the `ReadWriteLock` database, which it asks to be `0o600`, the same way. In a directory whose inheritable ACL grants other local accounts access to new files, those accounts can open the lock file or database. They can take the byte-range lock (a lock on a span of bytes in a file) once the owner releases it and keep the owner's later `acquire()` calls waiting or timing out. They can read holder metadata an `on_acquired` hook left in a `preserve_lock_file=True` file. Where the directory hands down modify access, they can write to both files. `lock.mode` still reports `0o600`. The documentation neither promises an ACL on Windows nor says that Windows drops the mode. On Unix the same mode keeps other accounts out. Mutual exclusion between processes that follow the protocol holds on either platform. Affected versions are filelock 3.10.0 through 4.2.0 on Windows. Anthropic's OSS Scanner reported this in private. Finding covered by this advisory: - `ANT-2026-0KEO0DDO` [medium, CWE-732]: WindowsFileLock ignores mode; 0o600 lock files inherit the directory ACL. ## Severity **CVSS 3.1 `CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:L/I:L/A:L` scores 5.3 (Medium).** `AV:L` (local): the other account needs a logon on the same host. `PR:L` (low privileges): an ordinary account is enough. `UI:N` (no user interaction). `AC:L` (low attack complexity): no race. `C:L` (partial confidentiality impact): the lock file (holder metadata) and the lock database are readable where `0o600` was meant to keep them private. `I:L` (partial integrity impact): in the directory layout the maintainers tested, the other account can also write both files, which lets it corrupt the lock's own state but nothing the lock protects. `A:L` (partial availability impact): the other account can hold the byte-range lock and make the owner wait or time out. `S:U` (scope unchanged). Classification from the review: **Windows permission failure with security impact only where callers rely on `mode=0o600` across accounts**. The review had marked it an unconfirmed Windows claim, since the scanner traced stand-in Win32 calls on Linux. The maintainers have since confirmed it on Windows with two local accounts (see Evidence). The scanner rated this medium; the maintainers keep medium. ## Scope and preconditions - The affected classes are `FileLock` and `AsyncFileLock` on Windows (`WindowsFileLock`), and the SQLite database of `ReadWriteLock` and `AsyncReadWriteLock`. Both go through `_open_non_reparse_fd`. The soft locks create their markers with `os.open`, which also keeps only a read-only bit of the mode on Windows; the scanner did not examine them further. - The directory is one that other local accounts cannot write, which the documentation already requires on Windows, but whose inheritable entries grant them read (or modify) access to new files. Examples are a service's data directory readable by `Users`, or any directory with `(OI)(CI)` entries (entries that new files and sub-directories inherit) for another account. A directory whose ACL already excludes other accounts, such as one inside a user profile, hides the problem. - The other account must open the file. According to the `LockFileEx` documentation, read access is enough to take the byte-range lock, so it can hold the lock whenever the owner does not. The lock file exists while the owner holds the lock, and stays after release with `preserve_lock_file=True`. The `ReadWriteLock` database is not unlinked at any point. - Unix is not affected, because `UnixFileLock` creates the file with the mode and repeats it with `fchmod`. Mutual exclusion between cooperating processes is not affected. ## Affected code Scanned commit `b831496b0273`: [`_open_non_reparse_fd` reduces the mode to one `read_only` bit](https://github.com/tox-dev/filelock/blob/b831496b0273ecd11bf02b8d080ca8b673a538c1/src/filelock/_windows.py#L252-L253). That bit is false for every accepted mode, because [`_check_mode`](https://github.com/tox-dev/filelock/blob/b831496b0273ecd11bf02b8d080ca8b673a538c1/src/filelock/_api.py#L526) rejects any mode without owner read and write, and an unset mode becomes `0o666`. In [`_nt_open`](https://github.com/tox-dev/filelock/blob/b831496b0273ecd11bf02b8d080ca8b673a538c1/src/filelock/_windows.py#L283-L310) the `OBJECT_ATTRIBUTES.SecurityDescriptor` field stays unassigned, and `read_only` only chooses `FILE_ATTRIBUTE_READONLY`. [`_open_lock_database`](https://github.com/tox-dev/filelock/blob/b831496b0273ecd11bf02b8d080ca8b673a538c1/src/filelock/_read_write.py#L1079-L1086) passes `0o600` through the same function on Windows. ## Introduced [d0061e4](tox-dev/filelock@d0061e4643df) "added multiuser support and associated tests" ([#192](tox-dev/filelock#192), 2023-03-15), first tag 3.10.0 (verified), added the `mode` parameter and on Windows handed it to `os.open`, which applies only the read-only attribute. The Windows backend has applied no more of the mode than that at any version, so this is a long-standing gap rather than a regression. ## Mechanism `_open_non_reparse_fd(path, mode)` computes `read_only = not mode & _OWNER_WRITE` and calls `_nt_open(path, read_only=read_only)`. `_nt_open` fills an `OBJECT_ATTRIBUTES` with the NT path and `OBJ_CASE_INSENSITIVE` and leaves `SecurityDescriptor` NULL. It then calls `NtCreateFile` with `FILE_OPEN_IF` and `FILE_ATTRIBUTE_NORMAL` (or `FILE_ATTRIBUTE_READONLY` when `read_only`, which no accepted mode produces). Nothing consults the group and other bits. Per the `NtCreateFile` and `OBJECT_ATTRIBUTES` documentation, a NULL descriptor gives the new file default security, which on NTFS is the set of inheritable entries of the parent directory. A lock file created with `mode=0o600` in a directory that hands down read access to `Users` is therefore readable by `Users`. Nothing tells the caller. The constructor validates the mode, `lock.mode` returns `0o600`, and `has_explicit_mode` is `True`. `docs/concepts.rst` says "filelock creates the file with it" and that "`0o600` and `0o700` keep other UIDs out", with no Windows caveat. Only the `_open_non_reparse_fd` docstring and a comment in `_soft.py` record the limitation. ## Reproduction The scanner's reproducer for `ANT-2026-0KEO0DDO`, quoted as received. It ran against commit `b831496b0273`. **`repro`** ```python #!/usr/bin/env python3 import os import shutil import subprocess import sys import tempfile private_job = "/tmp/repro.d/job_with_private_lock.py" # with FileLock(path, mode=0o600) default_job = "/tmp/repro.d/job_with_default_lock.py" # with FileLock(path) on_windows_backend = "/tmp/repro.d/run_on_windows_backend.py" # prints every Win32 call filelock makes os.umask(0o022) locks = tempfile.mkdtemp() # control: UnixFileLock, which leaves the lock file behind subprocess.run([sys.executable, private_job, f"{locks}/private.lock"], check=True) subprocess.run([sys.executable, default_job, f"{locks}/default.lock"], check=True) unix_private_mode = os.stat(f"{locks}/private.lock").st_mode & 0o777 unix_default_mode = os.stat(f"{locks}/default.lock").st_mode & 0o777 # trigger: the same two programs on WindowsFileLock, same lock name so the call lists can be compared whole windows_private = [sys.executable, on_windows_backend, private_job, f"{locks}/job.lock"] windows_default = [sys.executable, on_windows_backend, default_job, f"{locks}/job.lock"] windows_private_calls = subprocess.run(windows_private, check=True, capture_output=True, text=True).stdout.splitlines() windows_default_calls = subprocess.run(windows_default, check=True, capture_output=True, text=True).stdout.splitlines() shutil.rmtree(locks) # check: mode=0o600 must reach the OS; on Windows that means a security descriptor on NtCreateFile print(f"CONTROL UnixFileLock FileLock(path, mode=0o600): lock file mode {unix_private_mode:#o}") print(f"CONTROL UnixFileLock FileLock(path): lock file mode {unix_default_mode:#o}") for call in windows_private_calls: print(f"OBSERVED WindowsFileLock FileLock(path, mode=0o600): {call}") for call in windows_default_calls: print(f"OBSERVED WindowsFileLock FileLock(path): {call}") print(f"OBSERVED the two Win32 call lists are identical: {windows_private_calls == windows_default_calls}") honoured_on_unix = unix_private_mode == 0o600 and unix_default_mode == 0o644 private_create = next(call for call in windows_private_calls if call.startswith("NtCreateFile(")) dropped_on_windows = ( "ObjectAttributes.SecurityDescriptor=None" in private_create and windows_private_calls == windows_default_calls ) print("BUG" if honoured_on_unix and dropped_on_windows else "OK") ``` **`repro.d/job_with_default_lock.py`** ```python import sys from filelock import FileLock with FileLock(sys.argv[1]): pass ``` **`repro.d/job_with_private_lock.py`** ```python import sys from filelock import FileLock with FileLock(sys.argv[1], mode=0o600): pass ``` **`repro.d/run_on_windows_backend.py`** ```python import ctypes import fcntl import os import runpy import sys import types # Stand-ins for the ntdll, kernel32 and msvcrt functions filelock's Windows backend calls. Each prints the call with # the arguments filelock passed, then does the nearest Linux thing so the program runs to its end. def RtlDosPathNameToNtPathName_U_WithStatus(dos_path, nt_name, _file_part, _relative_name): print(f"RtlDosPathNameToNtPathName_U_WithStatus({os.path.basename(dos_path)!r})") nt_name._obj.Buffer = os.path.abspath(dos_path) return 0 # STATUS_SUCCESS def NtCreateFile( handle, access, attributes, _io_status, allocation_size, file_attributes, share, disposition, options, ea, ea_length ): object_attributes = attributes._obj print( f"NtCreateFile(DesiredAccess={access:#x}," f" ObjectAttributes.Attributes={object_attributes.Attributes:#x}," f" ObjectAttributes.SecurityDescriptor={object_attributes.SecurityDescriptor}," f" AllocationSize={allocation_size}, FileAttributes={file_attributes:#x}, ShareAccess={share:#x}," f" CreateDisposition={disposition}, CreateOptions={options:#x}, EaBuffer={ea}, EaLength={ea_length})" ) path = object_attributes.ObjectName.contents.Buffer handle._obj.value = os.open(path, os.O_RDWR | os.O_CREAT | os.O_NOFOLLOW) return 0 # STATUS_SUCCESS def RtlFreeUnicodeString(_nt_name): print("RtlFreeUnicodeString()") def GetFileInformationByHandle(_handle, info): print("GetFileInformationByHandle()") info._obj.dwFileAttributes = 0x80 # FILE_ATTRIBUTE_NORMAL: not a reparse point return True def LockFileEx(handle, flags, _reserved, bytes_low, bytes_high, _overlapped): print(f"LockFileEx(dwFlags={flags:#x}, nNumberOfBytesToLockLow={bytes_low}, nNumberOfBytesToLockHigh={bytes_high})") fcntl.flock(handle, fcntl.LOCK_EX) return True def UnlockFileEx(handle, _reserved, bytes_low, bytes_high, _overlapped): print(f"UnlockFileEx(nNumberOfBytesToUnlockLow={bytes_low}, nNumberOfBytesToUnlockHigh={bytes_high})") fcntl.flock(handle, fcntl.LOCK_UN) return True # only a filelock that builds a security descriptor for the lock file calls this def ConvertStringSecurityDescriptorToSecurityDescriptorW(sddl, _revision, descriptor, _size): print(f"ConvertStringSecurityDescriptorToSecurityDescriptorW({sddl!r})") descriptor._obj.value = 1 # any address but NULL return True # filelock sets argtypes on these at import; a lock taken without contention never calls them def not_called(*_arguments): raise NotImplementedError dlls = { "ntdll": types.SimpleNamespace( RtlDosPathNameToNtPathName_U_WithStatus=RtlDosPathNameToNtPathName_U_WithStatus, NtCreateFile=NtCreateFile, RtlFreeUnicodeString=RtlFreeUnicodeString, RtlNtStatusToDosError=not_called, ), "kernel32": types.SimpleNamespace( GetFileInformationByHandle=GetFileInformationByHandle, LockFileEx=LockFileEx, UnlockFileEx=UnlockFileEx, CloseHandle=not_called, OpenProcess=not_called, GetProcessTimes=not_called, ), "advapi32": types.SimpleNamespace( ConvertStringSecurityDescriptorToSecurityDescriptorW=ConvertStringSecurityDescriptorToSecurityDescriptorW, ), } # Import filelock once on the real platform, so the standard library modules it uses load their Linux versions, then # drop it and import it again with the Windows branches active. A HANDLE is the Linux descriptor number. import filelock # noqa: E402 for name in [name for name in sys.modules if name.split(".")[0] == "filelock"]: del sys.modules[name] ctypes.WinDLL = lambda name, use_last_error=False: dlls[name] sys.modules["msvcrt"] = types.SimpleNamespace(open_osfhandle=lambda handle, _flags: handle, get_osfhandle=lambda fd: fd) os.O_NOINHERIT = 0 sys.platform = "win32" import filelock # noqa: E402, F811 assert filelock.FileLock is filelock.WindowsFileLock sys.argv = sys.argv[1:] runpy.run_path(sys.argv[0], run_name="__main__") ``` ## Failure sequence ```mermaid sequenceDiagram participant O as owner process: FileLock(path, mode=0o600) participant N as NtCreateFile participant F as lock file (inherits the directory ACL) participant U as other local account O->>N: _open_non_reparse_fd(path, 0o600): read_only=False, SecurityDescriptor=NULL rect rgb(255, 214, 214) N-->>F: created with the parent's inheritable entries only, group and other bits not consulted Note over F: the file reports mode 0o600 to the owner, the ACL says otherwise end O->>F: LockFileEx (holds the lock), release, unlock U->>F: open for read (and write, where the directory hands down modify) U->>F: LockFileEx through that handle O->>F: acquire() -> waits or Timeout while U holds the byte-range lock U->>F: read holder metadata left by on_acquired with preserve_lock_file=True ``` ## Evidence The scanner's reproducer runs on a Linux image. It imports filelock's unmodified Windows backend with `sys.platform == "win32"` over stand-ins that print every Win32 call. The program with `mode=0o600` and the one without produce identical call lists (`RtlDosPathNameToNtPathName_U_WithStatus`, `NtCreateFile` with `FileAttributes=0x80` and `ObjectAttributes.SecurityDescriptor=None`, `RtlFreeUnicodeString`, `GetFileInformationByHandle`, `LockFileEx`, `UnlockFileEx`), while the Unix control produced `0o600` and `0o644` files. It prints `OBSERVED the two Win32 call lists are identical: True` and `BUG`. The scanner stated that it inferred the resulting ACL, and what another user can then do, from the documentation and did not run on Windows. Maintainer triage: the stand-in reproducer prints `BUG` on the scanned commit `b831496` and on the 4.2.0 release image. Maintainer verification on Windows 11 with two local accounts: a script creates `private.lock` with `FileLock(mode=0o600, preserve_lock_file=True)`, `default.lock` without a mode, and a `ReadWriteLock` database `lock.db` as the first account, in a directory granted `(OI)(CI)(M)` to a second account that is a member of `Users`. It then opens each file under the second account's logon token with `LogonUserW` and `ImpersonateLoggedOnUser`. On 4.2.0 the second account could open `private.lock` and `lock.db` for both read and write, the same as `default.lock`, and `icacls` listed only inherited `(I)` entries on all three. This confirms the claim the review had marked unconfirmed. ## Impact A program that passes `mode=0o600` so that other accounts cannot open its lock file gets no such restriction on Windows. In a directory that hands down read access, another account can hold the lock and make the owner wait or time out, and read holder metadata in a preserved lock file. Where the directory hands down modify access, as in the maintainers' test, it can also write the lock file and the `ReadWriteLock` database. With the mode honoured, as on Unix, a user without write access to the directory can do none of this. The exposure ends there: the effect reaches only the lock file or lock database, and mutual exclusion between processes that follow the protocol still holds. ## Workarounds - On Windows, keep lock files and `ReadWriteLock` databases in a directory whose ACL already excludes other accounts (inside the user profile, or a directory with inheritance disabled and only the owner, `SYSTEM` and `Administrators` granted), rather than relying on `mode`. - After creating a lock file with `preserve_lock_file=True` or a `ReadWriteLock` database, set its ACL yourself, for example `icacls <file> /inheritance:r /grant:r "<owner>:F" "SYSTEM:F" "Administrators:F"`; a file that exists keeps its ACL on later opens. - Do not store anything confidential in the lock file through `on_acquired` on Windows. ## Patched version 4.2.1. Fix: tox-dev/filelock#835 ## Credits Reported by Anthropic's OSS Scanner (automated analysis), 15 October 2026; reviewed by the maintainers. - 2026-10-15 23:08 UTC: report received (36 findings; this one rated medium by the scanner) - 2026-10-16 05:38 UTC: draft advisory created (first as part of GHSA-69hj-f68j-2879, then split out); two-account verification on Windows by the maintainers - 2026-10-16 01:16 UTC: fix PR #835 opened - Aliases: CVE-2026-112910 - Recommendation: 4.2.1 - `UnixFileLock`, the backend of `FileLock` and `AsyncFileLock` on Unix, empties whatever regular file it finds at the lock path once it holds the `flock` (a kernel lock on an open file). With an explicit `mode` it also chmods that file (sets its permission bits), whether or not this acquire created it. Another local user who can rename or hard-link one of the files of the user running the lock (below: the lock user) onto the lock path gets that file truncated by the lock user's next `acquire()`. A hard link is a second name for the same file. With a mode such as `0o666` the file, still owned by the lock user, also becomes readable and writable by that other user. The other user can then read and alter everything the lock user writes to it afterwards. `O_NOFOLLOW`, the defence added for the symlink variant (CVE-2025-68146), does not see a renamed file or a hard link. The rename route needs the file's directory to be writable without the sticky bit (a directory flag that lets only a file's owner remove or rename it). The hard-link route into a sticky directory such as `/tmp` needs `fs.protected_hardlinks=0`, a Linux kernel setting. Affected versions are every filelock release up to and including 4.2.0 (the truncation is as old as the project; the chmod since 3.10.2). Anthropic's OSS Scanner reported this in private. Finding covered by this advisory: - `ANT-2026-KO573NWI` [high, CWE-706]: UnixFileLock truncates and chmods victim files renamed onto the lock path. ## Severity **CVSS 3.1 `CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:L/I:H/A:N` scores 6.1 (Medium).** `AV:L` (local): the other user needs an account on the same host. `PR:L` (low privileges): an ordinary account is enough. `UI:N` (no user interaction). `AC:L` (low attack complexity): one rename or link, then wait for the lock user's next acquire; no race. `I:H` (high integrity impact): the lock user's own process truncates a file it did not mean to touch, including content behind a second name the other user could neither modify nor delete. With a permissive explicit `mode` the other user can then write to it. If that file is one the lock user later executes or trusts, that is code execution as the lock user. `C:L` (partial confidentiality impact): the old content is gone before the other user can read anything, but what the lock user writes afterwards through a descriptor (the number the kernel gives a process for an open file) it still holds is readable. `A:N` (no availability impact). `S:U` (scope unchanged). Classification from the review: **security, conditional** on another user controlling names in a non-sticky directory on the lock's filesystem (or on `fs.protected_hardlinks=0`). The scanner rated this high. The maintainers score it medium because the reachable targets depend on the directory layout and kernel settings described below. ## Scope and preconditions - Lock classes: `FileLock` and `AsyncFileLock` on Unix (`UnixFileLock`). `SoftFileLock` applies an explicit mode only to a file it created with `O_CREAT | O_EXCL` and does not truncate an existing file. `WindowsFileLock` has not truncated at any version. - The other user must put a regular file of the lock user at the lock path before the lock user's next acquire, while the lock file does not exist yet. That is the case on first use, after a reboot, after a `/tmp` cleaner, or after a release with the default `preserve_lock_file=False`, which unlinks the file. The scanner checked three ways at the scanned commit, with default kernel settings unless stated: - rename inside a directory without the sticky bit (a group-writable project or cache directory, a root-created `0777` directory); - rename from such a directory into a sticky one (`1777`) on the same filesystem. The sticky bit only protects entries that already exist, so the move succeeds while the lock path is free. Once the lock user's lock file exists there, the same move fails with `EPERM`; - `link(2)` into a sticky directory. This needs a system that lets a user hard-link a file it cannot write, which on Linux means `fs.protected_hardlinks=0`. Mainstream distributions set 1, which refuses the link. The scanner did not test macOS or the BSDs. - What the lock user loses: with the default `FileLock(path)` the lock truncates the file, which keeps its mode. With an explicit `mode` (a lock shared between users needs `0o666` or `0o660`, because `UnixFileLock` opens the file read-write), the lock also chmods the file to that mode. `_apply_explicit_mode` suppresses `PermissionError` only for files the lock user does not own. The lock user owns its own files, so the `fchmod` succeeds. - Trust boundary: the protection against another local user holds only when no other user can rename entries in the lock's directory or, for the rename-into-sticky route, in any directory on the same filesystem that holds a file of the lock user. A user who can rename entries in a non-sticky directory above the lock controls every name below it, which no file-based lock can defend against. - Not affected: a symlink (a file that points at another path) at the lock path, because `O_NOFOLLOW` makes the open fail with `ELOOP`; mutual exclusion between cooperating processes. ## Affected code Scanned commit `b831496b0273`: [`UnixFileLock._finalize_locked_fd`](https://github.com/tox-dev/filelock/blob/b831496b0273ecd11bf02b8d080ca8b673a538c1/src/filelock/_unix.py#L144-L163), whose [`os.ftruncate` and `_apply_explicit_mode` calls](https://github.com/tox-dev/filelock/blob/b831496b0273ecd11bf02b8d080ca8b673a538c1/src/filelock/_unix.py#L150-L154) have only the `st_nlink != 0` guard (the file still has at least one name); [`_apply_explicit_mode`](https://github.com/tox-dev/filelock/blob/b831496b0273ecd11bf02b8d080ca8b673a538c1/src/filelock/_unix.py#L165-L168); [`_open_lock_file`](https://github.com/tox-dev/filelock/blob/b831496b0273ecd11bf02b8d080ca8b673a538c1/src/filelock/_unix.py#L95-L121), which learns from its `O_EXCL` create whether this attempt made the file but hands that flag only to the `ENOSYS` fallback. ## Introduced The truncation of whatever is at the lock path dates from the initial commit ([2157efb](tox-dev/filelock@2157efb), which opened with `O_TRUNC`). The chmod of a pre-existing inode (the kernel's identity for a file, independent of its name) came with [d7e1e84](tox-dev/filelock@d7e1e84e71ad) "changed from os.umask to os.chmod" ([#206](tox-dev/filelock#206), 2023-03-22), first tag 3.10.2 (verified with `git tag --contains`). Before it the code passed the mode only to `open()`, which applies it to a file this call creates. The advisory range is therefore every release up to 4.2.0. The earlier group range `>= 3.10.0` understated it, as the review noted. ## Mechanism `_open_lock_file` first tries `os.open(path, O_RDWR | O_NOFOLLOW | O_CREAT | O_EXCL, mode)`. With the lock user's file renamed to the lock path that fails with `FileExistsError`. The second `os.open(path, O_RDWR | O_NOFOLLOW)` succeeds, because the entry is a regular file the lock user owns, and returns `created = False`. `_acquire_native` passes `created` only to the `ENOSYS` branch. No process holds a `flock` on the renamed file, so `_lock_fd_nonblocking(fd)` succeeds at once and `_finalize_locked_fd(fd)` runs: `os.fstat`, `st_nlink != 0`, `os.ftruncate(fd, 0)`, then `os.fchmod(fd, mode)` when the lock has an explicit mode. Nothing in that path tells a lock file from any other regular file; a link count of 2 and a non-empty file both pass. `O_NOFOLLOW` does not help because the entry is the file itself (or a hard link, an ordinary directory entry for the same inode), not a symlink to it. There is no race: the other user renames or links once and waits for the next acquire. `AsyncUnixFileLock` inherits `_finalize_locked_fd` unchanged. The scanner measured three variants at the same commit, same accounts (`victim` uid 1000, `attacker` uid 1001). First, a rename within a `0777` directory: the next `FileLock("jobs.lock", mode=0o666)` of `victim` empties the `0600` journal of `victim` renamed to `jobs.lock` and sets it to `0666`; `attacker` then reads what `victim` keeps writing through its open descriptor. Second, a rename of a two-name file (`ln /tmp/cache/blob ~/env/blob`, `0644`) from a `0777` directory, `fs.protected_hardlinks=1`: after `mv /tmp/cache/blob /tmp/cache/app.lock` and `FileLock("/tmp/cache/app.lock", mode=0o666)` as `victim`, `~/env/blob` has `2 links, mode 666, 0 bytes` and `echo pwned >> /home/victim/env/blob` as `attacker` succeeds, in a directory `attacker` cannot write. Third, `os.link(notes, lock_path)` in a `1777` directory with `fs.protected_hardlinks=0`: `notes.txt` (`0644`, in the home of `victim`) ends up 0 bytes with mode `0666`, and `open(notes, "w")` as `attacker` then succeeds. With plain `FileLock(lock_path)` a `0600` file ends up empty with its mode unchanged. The planted name stays after release, so every later acquire truncates the file again. ## Reproduction The scanner's reproducer for `ANT-2026-KO573NWI`, quoted as received. It ran against commit `b831496b0273`. **`repro`** ```python #!/usr/bin/env python3 import os import secrets import shutil import stat import subprocess import sys # set up: a directory every user may write, without the sticky bit shared = "/tmp/renamed-lock-file-demo" shutil.rmtree(shared, ignore_errors=True) os.mkdir(shared) os.chmod(shared, 0o777) journal = f"{shared}/victim-journal.log" lock_path = f"{shared}/jobs.lock" victim = subprocess.Popen( ["runuser", "-u", "victim", "--", sys.executable, "/tmp/repro.d/victim_job_runner.py", shared], stdin=subprocess.PIPE, stdout=subprocess.PIPE, text=True, ) def victim_runs(job): victim.stdin.write(job + "\n") victim.stdin.flush() assert victim.stdout.readline() == "done\n" def as_attacker(script, argument): command = ["runuser", "-u", "attacker", "--", sys.executable, f"/tmp/repro.d/{script}", argument] return subprocess.run(command, capture_output=True, text=True, check=True).stdout # before the attack: one job leaves the lock file and a 0600 journal with one line in it victim_runs("job 1") journal_before = os.stat(journal) journal_content_before = open(journal).read() attacker_read_before = as_attacker("attacker_read_file.py", journal) print( f"CONTROL journal: inode {journal_before.st_ino}, uid {journal_before.st_uid}, " f"mode {stat.S_IMODE(journal_before.st_mode):o}, content {journal_content_before!r}" ) print(f"CONTROL attacker reads the journal: {attacker_read_before!r}") # attack as_attacker("attacker_rename_journal.py", shared) # the victim takes its lock again, then notes a secret in its journal through the descriptor it holds secret = secrets.token_hex(8) victim_runs(f"job 2 {secret}") victim.stdin.close() victim.wait() # check: the victim's private journal must not be emptied, widened to 0666, or readable by the attacker lock_after = os.stat(lock_path) lock_content_after = open(lock_path).read() attacker_read_after = as_attacker("attacker_read_file.py", lock_path) print( f"OBSERVED lock path: inode {lock_after.st_ino}, uid {lock_after.st_uid}, " f"mode {stat.S_IMODE(lock_after.st_mode):o}, content {lock_content_after!r}" ) print(f"OBSERVED attacker reads the lock path: {attacker_read_after!r}") attacker_was_denied = attacker_read_before == "permission denied\n" lock_path_is_the_journal = (lock_after.st_dev, lock_after.st_ino) == (journal_before.st_dev, journal_before.st_ino) journal_was_emptied = "job 1" in journal_content_before and "job 1" not in lock_content_after journal_was_widened = stat.S_IMODE(journal_before.st_mode) == 0o600 and stat.S_IMODE(lock_after.st_mode) == 0o666 attacker_reads_the_secret = secret in attacker_read_after bug = ( attacker_was_denied and lock_path_is_the_journal and journal_was_emptied and journal_was_widened and attacker_reads_the_secret ) print("BUG" if bug else "OK") ``` **`repro.d/attacker_read_file.py`** ```python import sys try: print(open(sys.argv[1]).read(), end="") except PermissionError: print("permission denied") ``` **`repro.d/attacker_rename_journal.py`** ```python import os import sys shared_directory = sys.argv[1] # needs only write access to the directory: no sticky bit, so any user may rename any entry in it os.rename(f"{shared_directory}/victim-journal.log", f"{shared_directory}/jobs.lock") ``` **`repro.d/victim_job_runner.py`** ```python import os import sys from filelock import FileLock shared_directory = sys.argv[1] journal_fd = os.open(f"{shared_directory}/victim-journal.log", os.O_WRONLY | os.O_CREAT | os.O_APPEND, 0o600) # UnixFileLock opens the lock file O_RDWR, so a lock that other users of the directory also take needs mode=0o666 lock = FileLock(f"{shared_directory}/jobs.lock", mode=0o666, timeout=5) for job in sys.stdin: with lock: os.write(journal_fd, f"ran {job}".encode()) print("done", flush=True) ``` ## Attack sequence ```mermaid sequenceDiagram participant A as other account participant D as shared directory (writable, not sticky) participant V as lock user: FileLock("jobs.lock", mode=0o666) participant K as kernel Note over V,D: victim-journal.log, owner victim, mode 0600 (the other account cannot read it) A->>D: mv victim-journal.log jobs.lock (one rename, no race) V->>K: open(jobs.lock, O_RDWR|O_NOFOLLOW|O_CREAT|O_EXCL) -> EEXIST V->>K: open(jobs.lock, O_RDWR|O_NOFOLLOW) -> fd (regular file, created=False) V->>K: flock(fd, LOCK_EX|LOCK_NB) -> ok (no holder) rect rgb(255, 214, 214) V->>K: fstat(fd), st_nlink != 0 V->>K: ftruncate(fd, 0) V->>K: fchmod(fd, 0o666) Note over V,K: the journal's inode is now empty and world read-write, still owned by victim end V->>K: write("ran job 2 <secret>") through the journal descriptor it still holds A->>D: read jobs.lock -> "ran job 2 <secret>" ``` ## Evidence The scanner reproducer (`victim` and `attacker` accounts, root-created `0777` directory without the sticky bit under `/tmp`, default sysctls, Linux 6.1, CPython 3.14.8, overlayfs; lock `FileLock(path, mode=0o666, timeout=5)`). It printed `CONTROL journal: inode 296383, uid 1000, mode 600, content 'ran job 1\n'` and `CONTROL attacker reads the journal: 'permission denied\n'`. After the rename and the second job of `victim` it printed `OBSERVED lock path: inode 296383, uid 1000, mode 666, content 'ran job 2 f4bc9a4e04c3f556\n'` and `OBSERVED attacker reads the lock path: 'ran job 2 f4bc9a4e04c3f556\n'`, last line `BUG`. The verdict requires that the earlier read by `attacker` failed, that the lock path is the journal's inode, that the old line is gone, that the mode went from `0600` to `0666`, and that the later read by `attacker` contains the secret. It runs in under a second with no race. The scanner ran the two-name and `link(2)` variants above by hand at the same commit (commands and results in its report), not through the script. Maintainer triage: the reproducer prints `BUG` on the scanned commit `b831496` and on the 4.2.0 release image. The independent review (`soft-open`) confirmed the mechanism and that the hard-link and two-name variants go through the same two calls. ## Impact The file's content is lost in every configuration, including for processes that hold the file open and under every other name the file has. With an explicit permissive `mode`, the other user can read and modify everything the lock user writes afterwards, while the file still belongs to the lock user. The other user can rename the file back to its old name after the acquire. It stays there with the lock's mode, and the lock user's next acquire creates a fresh lock file. Somebody who may rename a single-name file may also unlink it, so losing such a file under its name is not new. What the lock adds are two operations on the inode that the other user cannot perform, `ftruncate` and `fchmod`, run with the lock user's privileges. Those matter for a descriptor the lock user still holds, for a second name of the file, and for programs that trust the file's ownership. The other user does not choose the mode, and the lock user writes no bytes the other user chose. Mutual exclusion is not affected. ## Workarounds - Keep the lock file in a directory no other account can write, or in a sticky directory on a filesystem where no directory holding the lock user's files is writable by others. On Linux keep `fs.protected_hardlinks=1` (the default on mainstream distributions), which closes the hard-link route. - Do not pass a permissive explicit `mode` (`0o666`, `0o660`, `0o644`) unless accounts share the lock; without a mode the exposure is the truncation only. - Keep the lock file in place (`preserve_lock_file=True`). Once a lock file owned by the lock user exists in a sticky directory, another user cannot rename or link over it. - Where the lock does not need `flock` semantics, `SoftFileLock` applies its mode only to the marker it created with `O_EXCL` and does not truncate an existing file. ## Patched version 4.2.1. Fix: tox-dev/filelock#835 ## Credits Reported by Anthropic's OSS Scanner (automated analysis), 15 October 2026; reviewed by the maintainers. - 2026-10-15 23:08 UTC: report received (36 findings; this one rated high by the scanner) - 2026-10-16 00:46 UTC: draft advisory created (first as a group of four findings, then narrowed to `ANT-2026-KO573NWI`; the other three security findings have their own advisories) - 2026-10-16 01:16 UTC: fix PR #835 opened - Aliases: CVE-2026-112909 - Recommendation: 4.2.1

gaborbernat requested changes Mar 12, 2023

View reviewed changes

gaborbernat requested changes Mar 13, 2023

View reviewed changes

gaborbernat requested changes Mar 14, 2023

View reviewed changes

gaborbernat reviewed Mar 15, 2023

View reviewed changes

Comment thread src/filelock/_api.py Resolved

gaborbernat reviewed Mar 15, 2023

View reviewed changes

gaborbernat requested changes Mar 15, 2023

View reviewed changes

jahrules and others added 10 commits March 15, 2023 15:59

added multiuser support and associated tests

c0551fe

[pre-commit.ci] auto fixes from pre-commit.com hooks

a5e6517

for more information, see https://pre-commit.ci

fixed a bug in windows File Perms

080b8d7

flake8 fixes

a7bae6f

fixed test for windows condition

98695dd

adding filemode to whitelist

6bb1cff

added changelog as requested

acf338b

updated code based on comments and added tests for os.umask()

ea1e757

updates as requested

31f634a

changes as requested

4259eae

updated os.umask to be inside a try:finally

b4d42bb

gaborbernat approved these changes Mar 15, 2023

View reviewed changes

gaborbernat merged commit d0061e4 into tox-dev:main Mar 15, 2023

chriskuehl mentioned this pull request Mar 22, 2023

filelock 3.10.0 changes umask to 0 in multithreaded processes, causing world-writable files to be created #204

Closed

hmaarrfk mentioned this pull request Oct 30, 2023

Allow users to subclass FileLock with custom keyword arguments #284

Merged

gaborbernat mentioned this pull request Oct 16, 2026

🐛 fix(open): keep lock files intact, honor deadlines and mode #835

Merged

amine-malloul-gira mentioned this pull request Oct 29, 2026

Splat Dependency Updates gira-de/splat#35

Merged

github-actions Bot mentioned this pull request Nov 6, 2026

[candidate] CVE-2026-112910 (via GHSA-8mx5-7m5m-r77g) patrickmgarrity/Anthropic-Credited-CVEs#792

Merged

Conversation

jahrules commented Feb 2, 2023

Uh oh!

jahrules commented Feb 20, 2023

Uh oh!

jahrules commented Feb 26, 2023

Uh oh!

gaborbernat left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jahrules commented Mar 13, 2023 via email

Uh oh!

jahrules commented Mar 13, 2023 via email

Uh oh!

gaborbernat commented Mar 14, 2023

Uh oh!

jahrules commented Mar 14, 2023 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jahrules Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaborbernat left a comment

Choose a reason for hiding this comment

Uh oh!

jahrules commented Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jahrules Mar 15, 2023 •

edited

Loading

jahrules commented Mar 15, 2023 •

edited

Loading