Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to overwrite without race condition #335

Merged
merged 3 commits into from
Nov 12, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 10 additions & 47 deletions cachecontrol/caches/file_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import hashlib
import os
import tempfile
from textwrap import dedent
from typing import IO, TYPE_CHECKING, Union
from pathlib import Path
Expand All @@ -18,47 +19,6 @@
from filelock import BaseFileLock


def _secure_open_write(filename: str, fmode: int) -> IO[bytes]:
# We only want to write to this file, so open it in write only mode
flags = os.O_WRONLY

# os.O_CREAT | os.O_EXCL will fail if the file already exists, so we only
# will open *new* files.
# We specify this because we want to ensure that the mode we pass is the
# mode of the file.
flags |= os.O_CREAT | os.O_EXCL

# Do not follow symlinks to prevent someone from making a symlink that
# we follow and insecurely open a cache file.
if hasattr(os, "O_NOFOLLOW"):
flags |= os.O_NOFOLLOW

# On Windows we'll mark this file as binary
if hasattr(os, "O_BINARY"):
flags |= os.O_BINARY

# Before we open our file, we want to delete any existing file that is
# there
try:
os.remove(filename)
except OSError:
# The file must not exist already, so we can just skip ahead to opening
pass

# Open our file, the use of os.O_CREAT | os.O_EXCL will ensure that if a
# race condition happens between the os.remove and this line, that an
# error will be raised. Because we utilize a lockfile this should only
# happen if someone is attempting to attack us.
fd = os.open(filename, flags, fmode)
try:
return os.fdopen(fd, "wb")

except:
# An error occurred wrapping our FD in a file object
os.close(fd)
raise


class _FileCacheMixin:
"""Shared implementation for both FileCache variants."""

Expand Down Expand Up @@ -122,15 +82,18 @@ def _write(self, path: str, data: bytes) -> None:
Safely write the data to the given path.
"""
# Make sure the directory exists
try:
os.makedirs(os.path.dirname(path), self.dirmode)
except OSError:
pass
dirname = os.path.dirname(path)
os.makedirs(dirname, self.dirmode, exist_ok=True)

with self.lock_class(path + ".lock"):
# Write our actual file
with _secure_open_write(path, self.filemode) as fh:
fh.write(data)
(fd, name) = tempfile.mkstemp(dir=dirname)
try:
os.write(fd, data)
finally:
os.close(fd)
os.chmod(name, self.filemode)
os.replace(name, path)

def _delete(self, key: str, suffix: str) -> None:
name = self._fn(key) + suffix
Expand Down
Loading