[security] C-4: Pickle deserialization of database/Redis data

[jacobyoby]

Severity: Critical
Files: fixpickle.py:11,13, encryption.py:57-58,99-101

Problem

pickle.loads() deserializes data from Redis (sessions) and database (ML entries, user dicts). DB/Redis compromise leads to remote code execution. unpack_object() and unpack_dictionary() deserialize without encryption.

Remediation

Replace pickle with JSON for new data. Add HMAC integrity verification for existing pickle data. Migrate stored pickle data to JSON format.

Source

Finding from security audit (2026-08-31). See .github/SECURITY_AUDIT_TRIAGE.md.

[jacobyoby]

Assessment 2026-09-10: the remaining #32 work is the planned JSON+HMAC migration, which is architectural — not a one-PR surgical fix — so it stays open with this scoping note. Verified current defense stack in-tree: (1) RestrictedUnpickler denylist on every stored-data load (fixpickle.py:43, blocks classic RCE primitives incl. aliased-module re-check); (2) safe_pickle() strips modules/functions on write plus pickleable_objects allowlist; (3) AES-GCM AEAD envelopes for all new encryptions (#40, 2026-09-09). Why the remainder cannot be sliced surgically: HMAC on new writes with legacy fallback is theater against the stated DB-writer threat (attacker simply writes legacy-format rows, which must keep loading); removing the legacy CBC fallback now would kill all sessions predating yesterday's GCM cutover; pack_dictionary still writes unencrypted base64 pickle to session stores. The real project: versioned envelopes, a JSON schema covering session object types, dual-read with backfill tooling, a legacy sunset policy, and upstream coordination (format fork risk) — a designed multi-step effort with data-loss risk if rushed. Residual risk until then: a DB/Redis writer is constrained by the denylist (self-described speed bump, copyreg still allowed) plus GCM integrity on new secrets. Recommend scheduling the migration as its own project, not a drive-by.

[jacobyoby]

Re-verified 2026-09-10 on origin/jacob/maintained (post-#106): the scoping note above still holds and no clean surgical slice exists. Confirmed in-tree: (1) RestrictedUnpickler denylist + resolved-object re-check in docassemble_webapp/docassemble/webapp/utils/fixpickle.py, consumed by fix_pickle_obj/fix_pickle_dict on session/ML loads; (2) safe_pickle() module/function stripping in docassemble_webapp/docassemble/webapp/utils/encryption.py:111; (3) AES-GCM $GCM1$ envelopes for new encryptions with CBC legacy fallback in the same file; (4) pack_dictionary still writes unencrypted base64 pickle. Candidate drive-bys were rejected deliberately: denylist extension is theater against a DB-writer (copyreg must stay allowed) and risks breaking legit session loads; HMAC-on-write without a sunset is bypassed via legacy rows; dropping the CBC fallback or encrypting pack_dictionary is the migration itself with data-loss risk. No PR opened — this stays a scheduled multi-step project.