Skip to content

Stream OOXML decryption to avoid buffering the whole payload - #102

Open
poofeg wants to merge 1 commit into
nolze:masterfrom
poofeg:master
Open

poofeg wants to merge 1 commit into
nolze:masterfrom
poofeg:master

Conversation

@poofeg

@poofeg poofeg commented Aug 16, 2026

Copy link
Copy Markdown

Closes #101

Problem

OOXMLFile.decrypt() materialized the entire decrypted OOXML package in memory and copied it several times. Both ECMA376Agile.decrypt() and ECMA376Standard.decrypt() accumulated output in a BytesIO and returned a second full copy, and ooxml.py copied the result again into a BytesIO just to run zipfile.is_zipfile(). Peak RSS was ~4–5× the input size.

Changes

  • Agile (msoffcrypto/method/ecma376_agile.py): extracted the existing 4096-byte segment loop into a streaming generator ECMA376Agile.decrypt_stream(...). decrypt(...) is now a thin wrapper returning b"".join(...), preserving the old API.
  • Standard (msoffcrypto/method/ecma376_standard.py): added ECMA376Standard.decrypt_stream(...), which feeds AES-ECB incrementally in chunks and yields exactly totalSize plaintext bytes. decrypt(...) remains as a wrapper.
  • Validation (msoffcrypto/format/ooxml.py): replaced zipfile.is_zipfile(io.BytesIO(obuf)) with a tail-only end-of-central-directory check (_is_zipfile_tail) that keeps just the last 65,557 bytes while streaming, so the payload is never copied for validation. The same InvalidKeyError is raised on failure.
  • OLE stream read (msoffcrypto/format/ooxml.py): olefile.openstream() eagerly loads the whole EncryptedPackage stream into a BytesIO (see olefile's own FIXME), which dominated the remaining memory. Added _OleStreamReader to follow the FAT chain and read one sector at a time, with a fallback to openstream() for small/mini-FAT streams.

Backward compatibility

  • OOXMLFile.decrypt(outfile) and the top-level OfficeFile.decrypt(outfile) keep the same signatures.
  • ECMA376Agile.decrypt(...) and ECMA376Standard.decrypt(...) still return bytes.

Tests

  • Added tests/test_ooxml_streaming.py: verifies the streaming output is byte-identical to the buffered decrypt(), works with a non-seekable output destination, and that a wrong password still raises InvalidKeyError. No large binary fixtures are committed.
  • Full suite passes (pytest).

Memory

Measured with /usr/bin/time -l on a 1.79 GB agile-encrypted .ppsx:

  • before: ~4.95 GiB peak RSS
  • after: ~0.88 GiB peak RSS

The remaining ~0.88 GiB comes from olefile.OleFileIO.__init__ parsing the OLE structure (its loadfat builds the FAT with array + array in a loop), which runs before any decryption and is outside this patch. The streaming decrypt path itself now stays at a small constant buffer plus one 4096-byte segment.

Out of scope

verify_integrity=True (the agile HMAC path) still reads the full stream once, because it needs the entire payload to compute the HMAC. It is opt-in and left unchanged.

@nolze

nolze commented Aug 20, 2026

Copy link
Copy Markdown
Owner

Thank you for your valuable contribution!
I'd like to merge this, but since there is some overlap with the related PR in olefile, I'd prefer to wait a little while to see how that is handled there.
If it looks like that may take some time, I'd be happy to move forward with addressing it here instead.
I'm also considering keeping the existing implementation and adding the streaming implementation separately, rather than replacing the current one.

@poofeg

poofeg commented Aug 20, 2026

Copy link
Copy Markdown
Author

Thank you for your valuable contribution! I'd like to merge this, but since there is some overlap with the related PR in olefile, I'd prefer to wait a little while to see how that is handled there. If it looks like that may take some time, I'd be happy to move forward with addressing it here instead. I'm also considering keeping the existing implementation and adding the streaming implementation separately, rather than replacing the current one.

Thank you for the thoughtful review!

I've opened #103, which addresses the overlap concern directly: it's the same streaming implementation, but without the _OleStreamReader workaround. It relies on the lazy OleStream from the related olefile PR (decalage2/olefile#177) and reads EncryptedPackage directly via openstream(), so there's no duplicated OLE-stream handling on the msoffcrypto side anymore.

What I'd suggest:

I'm happy to wait and see how the olefile maintainer responds — if that stalls, we can always switch back to #102.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OOXML decrypt() holds the entire decrypted package in memory — add a streaming path

2 participants