Skip to content

Replace the pickle-backed data cache with JSON files - #569

Closed
frack113 wants to merge 2 commits into
mainfrom
fix/replace-diskcache-with-json-file-cache
Closed

frack113 wants to merge 2 commits into
mainfrom
fix/replace-diskcache-with-json-file-cache

Conversation

@frack113

Copy link
Copy Markdown
Member

Problem

sigma.data.mitre_attack and sigma.data.mitre_d3fend cached their downloaded data through diskcache.Cache, which pickles values by default. Anyone able to write into the cache directory could therefore execute code the next time a rule referenced the data:

  • CVE-2025-69872 / GHSA-w8v5-vhqr-4h9v (diskcache <= 5.6.3)
  • diskcache 5.6.3 is unmaintained and no patched release exists, so the only real fix is to drop the dependency.

The default cache directory is ~/.cache/pysigma/, created with mode 0o755, so it is not a hardened location either.

Change

Adds sigma.data.cache.JsonFileCache, a small key-value store holding one JSON document per key, and uses it in both loaders. It has no dependency outside the standard library.

  • JSON only. Values go through json.dumps/json.load, so a tampered entry can at worst produce inert data or a parse failure, which is treated as a miss. No deserialization of attacker-chosen types.
  • Atomic writes. Each set() writes to a temporary file in the cache directory then os.replace()s it into place, so a concurrent reader never observes a partial document and an interrupted write leaves no usable garbage.
  • Contained keys. The file name is sha256(key)[:32] + ".json", so any key stays inside the cache directory.
  • Tight permissions. Directory 0o700 (re-chmoded even when it already exists, since mkdir(exist_ok=True) leaves the mode alone), entries 0o600.
  • Self-healing. A corrupted, truncated or invalid-UTF-8 entry is reported as a miss and discarded so the caller re-fetches instead of failing.
  • Clean upgrade. clear() also removes the former store's cache.db, cache.db-wal, cache.db-shm and cache.db-journal, so an upgrade does not strand a stale database nobody will ever open again.

diskcache and diskcache-stubs are dropped from pyproject.toml and poetry.lock.

API

get, set, discard, clear and close mirror the small part of the diskcache.Cache surface the loaders actually used, so the loaders' behaviour is unchanged. close() is a no-op, kept for compatibility. A leading ~ is expanded.

Tests

  • tests/test_data_cache.py — round-trip, key containment and collision resistance, atomic write and cleanup on failure, corruption handling, permissions, cross-process reuse, concurrent writers, and a guard that the module and pyproject.toml no longer mention diskcache.
  • tests/test_mitre_data_loading.py — exercises both loaders against local fixture files: parsing, cache write, serving from disk with the source deleted and the network forbidden, clear_cache()/set_url()/set_cache_dir() behaviour, and recovery from a tampered entry.

The permission tests are skipped on Windows, where POSIX modes are not meaningful; the CI matrix also covers macOS.

Full suite: 1707 passed, 1 skipped. mypy (strict) reports the same 3 pre-existing unused-ignore errors and no new ones.

The MITRE ATT&CK and D3FEND loaders cached their downloaded data through
diskcache.Cache, which pickles values by default. Anything able to write
into the cache directory could then execute code the next time a rule
referenced the data, via CVE-2025-69872 (GHSA-w8v5-vhqr-4h9v). diskcache
5.6.3 is unmaintained and no patched release exists.

Add sigma.data.cache.JsonFileCache, a small key-value store that keeps one
JSON document per key, and use it in both loaders. Values are deserialized
with json only, so a tampered entry can at worst yield inert data or a parse
failure, which is treated as a miss. Entries are keyed by a SHA-256 digest so
any key stays inside the cache directory, and each write goes through a
temporary file and os.replace so a reader never observes a partial document.

clear() also removes the files of the former store so an upgrade does not
strand a stale database. This drops diskcache and diskcache-stubs.
expanduser() consults HOME on POSIX but USERPROFILE on Windows, so the test
failed on the Windows leg of the CI matrix.
@thomaspatzke

Copy link
Copy Markdown
Member

I don't see the criticality of the issue here that justifies a full implementation of a custom caching implementation. The cache is located in safe temporary locations and the exploitation implies that an attacker has write access to the libraries execution environment, which likely implies also code execution capabilities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants