Skip to content

Fix three bugs that silently drop or crash on page text extraction - #29

Open
jrzmurray wants to merge 1 commit into
DissectMalware:mainfrom
jrzmurray:fix-array-property-values-and-mac-text-extraction
Open

Fix three bugs that silently drop or crash on page text extraction#29
jrzmurray wants to merge 1 commit into
DissectMalware:mainfrom
jrzmurray:fix-array-property-values-and-mac-text-extraction

Conversation

@jrzmurray

Copy link
Copy Markdown

Summary

Three bugs in the property/file-node parsing that either crash outright or silently drop page text, found while extracting a real multi-year notebook with pages authored on both Windows and Mac OneNote clients.

  1. PropertyID.type == 0x10 (ArrayOfPropertyValues) is unimplemented (FileNode.py, PropertySet.__init__): it's a bare raise NotImplementedError. Any page using an array-typed property fails to parse at all, and since callers typically catch the exception around a whole-file parse (as the pyonenote CLI/most integrations do), this silently drops all text in that section. Implemented per [MS-ONESTORE] 2.6.9 prtArrayOfPropertyValues: a cProperties count, then (if nonzero) a prid header followed by that many nested PropertySet structures.

  2. ObjectDeclaration2LargeRefCountFND has no dispatch branch in FileNode.__init__. Used for objects whose reference count doesn't fit in one byte (larger/more heavily-shared content). Since no branch matches, self.data is never set for such nodes, which:

    • makes their content invisible to get_properties(), and
    • crashes with AttributeError: 'FileNode' object has no attribute 'data' if the node has baseType == 2 (i.e. has children), since the trailing self.children.append(FileNodeList(file, self.document, self.data.ref)) runs unconditionally.

    Also fixed: the two ReadOnlyObjectDeclaration2*RefCountFND branches set self.data but never parse out self.propertySet (unlike the plain ObjectDeclaration2RefCountFND branch does), so their text was likewise invisible even though the node itself didn't crash. OneDocment.get_properties()'s hardcoded single-type filter (['ObjectDeclaration2RefCountFND']) is broadened to include all four declaration node types, with the ReadOnly* variants' body accessed via their .base wrapper.

  3. TextExtendedAscii is decoded as UTF-16 instead of latin-1. OneNote for Mac stores plain-text runs under this single-byte-per-character property instead of Windows OneNote's RichEditTextUnicode (UTF-16). Decoding it as UTF-16 either garbles it into CJK-looking noise (even byte count) or throws and falls back to a raw hex dump (odd byte count) -- either way the original text is unrecoverable from get_properties()'s output. This one likely explains why pages/notebooks captured on OneNote for Mac often come back looking empty or garbled with this library.

Testing

No test suite exists in this repo to extend, so I verified against a real notebook backup with a mix of Windows- and Mac-authored sections, before/after this patch (content is private/client data so I can only share aggregate numbers, not excerpts):

SectionBeforeAfter
Section hitting the ArrayOfPropertyValues crashcrashed, 0 chars extracted~1.17M chars extracted
Another section w/ same issuecrashed, 0 chars extracted~611K chars extracted
Mac-authored section~330 bytes (bare page timestamps only, no body text)~173K chars of real body text extracted
Another Mac-authored sectiontitles only, no body text~336K chars of real body text extracted

Happy to add a synthetic .one fixture + test if that's useful for merging -- let me know your preference for how this project handles tests.

Compatibility

No public API changes. PropertySet(file) (single-arg, previously broken/unreachable for the 0x11 array-element branch) now consistently requires the same (file, OIDs, OSIDs, ContextIDs, document) signature used everywhere else it's constructed in this file, matching how PropertySet is actually always instantiated in practice.

- Implement PropertyID.type 0x10 (ArrayOfPropertyValues), which was a
bare `raise NotImplementedError`. Any section containing a page with
an array-typed property failed to parse at all, silently dropping
all text in that section.
- Handle ObjectDeclaration2LargeRefCountFND file nodes, which had no
matching branch in FileNode.__init__ at all. self.data was never
set for such nodes, which (a) made their content invisible to
get_properties() and (b) crashed with
"'FileNode' object has no attribute 'data'" if the node had
baseType == 2 (i.e. had children). Also load propertySet for the
two ReadOnlyObjectDeclaration2*RefCountFND variants, which set
self.data but never parsed out the property set, and broaden
OneDocment.get_properties()'s hardcoded single-type filter to
include all four declaration node types.
- Decode the TextExtendedAscii property as latin-1 instead of
utf-16. OneNote for Mac stores plain-text runs under this
single-byte-per-character property instead of Windows OneNote's
RichEditTextUnicode (UTF-16). Decoding it as utf-16 either garbles
it into CJK-looking noise (even byte count) or throws and falls
back to a raw hex dump (odd byte count) -- in both cases the
original text is unrecoverable from the output.
Tested against a real, multi-year OneNote notebook with pages
authored on both Windows and Mac OneNote clients. Sections that
previously extracted 0 characters (crashing on the ArrayOfPropertyValues
error) now extract hundreds of KB to over 1MB of real text each;
Mac-authored sections that previously returned only bare page
timestamps (no body text at all) now extract their full text content.
@jrzmurray

Copy link
Copy Markdown
Author

Heads up: after opening this, I found that the ArrayOfPropertyValues fix here overlaps with #16 by @ostraconify (open since March 2023), which implements the same property type. While integration-testing #16 I also found two bugs in its implementation (nested array elements were appended to self.rgData individually instead of as one list, which desyncs self.rgData from self.rgPrids for every property after an array-typed one; and the prid header was read unconditionally even when the spec says it must not be present for an empty array). I've opened #30 to finish #16 with proper attribution to @ostraconify and fix both of those.

So there's now duplication between this PR and #30 on the ArrayOfPropertyValues piece specifically. The other two fixes here -- the missing ObjectDeclaration2LargeRefCountFND dispatch branch (and ReadOnly* propertySet loading / get_properties() filter broadening that goes with it), and decoding TextExtendedAscii as latin-1 for OneNote-for-Mac content -- aren't covered by #16, #20, #26, #27, or #28, so those parts of this PR should still stand on their own regardless of what happens with #30.

Also worth noting for review: #20 and #28 both independently propose the same FileNode.data crash-guard fix that's part of this PR too (#28 is the cleanest version, with a regression test). And #26/#27 are unrelated, complementary defensive-parsing fixes. Happy to help however's most useful in sorting out merge order across all of these.

jrzmurray added a commit to jrzmurray/pyOneNote that referenced this pull request Jul 14, 2026
…odes
Builds on the page-text-extraction fixes in the preceding commit (PR DissectMalware#29),
which this feature depends on: without the TextExtendedAscii latin-1 decode,
Mac-authored sections are garbled by the existing UTF-16 decode before the
exporter ever sees them.
Markdown export (-m/--markdown):
- OneDocument.get_pages()/get_markdown() reconstruct de-duplicated page text.
OneNote keeps every historical revision of a page inline, so the raw property
stream repeats each page 2-20x; pages are segmented on jcidPageNode, titled
via their CachedTitleString, and collapsed to one entry each.
- Text runs are read per platform -- RichEditTextUnicode (Windows) and
TextExtendedAscii (OneNote for Mac) -- and joined into clean markdown, one
.md per section.
Redaction (-r/--redact-secrets), off by default:
- Replaces high-confidence credential shapes with visible [redacted:*] markers.
- Applies to ALL output modes -- the default dump, JSON (-j), markdown (-m), and
text attachments written to --output-dir. Binary attachments have no text to
scrub and are written unchanged.
- Matching is limited to prefix/structure-keyed vendor sentinels (AWS, GitHub,
GitLab, OpenAI/Anthropic, Slack, Stripe, Google, npm, PyPI, SendGrid, Twilio,
JWT, Bearer) and PEM PRIVATE KEY blocks. Public certificates, public keys, and
CSRs are shareable and deliberately left intact, and there is no generic
entropy/keyword/base64 detection, so ordinary note text and embedded data are
untouched.
tests/test_redact.py: stdlib-unittest coverage for full-span redaction, public
material preservation, nested-tree scrubbing, and binary passthrough.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for freeto 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.

1 participant

@jrzmurray