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

Ensure FileNode.data is initialized in __init__ and is not used if it… #20

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cccs-jh
Copy link

@cccs-jh cccs-jh commented Jul 13, 2023

… is None.

Currently if the file_node_type is not handled data is uninitialized and can cause an AttributeError.

@DissectMalware
Copy link
Owner

Thank you for the PR. Can you please give me a sample file so I can test it?

@cccs-jh
Copy link
Author

cccs-jh commented Jan 30, 2024

Unfortunately I can't share the sample that caused the error because it contains client data. I can share that the FileNode that caused the error had an invalid FileNodeID, and a BaseType of 2. The file actually has several FileNodes with invalid FileNodeIDs so fixing this issue probably wouldn't allow the file to be parsed correctly, but it would allow the constructor to complete. That would let the partially parsed results be inspected. Another option would be to raise an error with a meaningful error message on an invalid id, if returning a best approximation on a parsing failure isn't a project goal.

Let me see if I can find anything more useful about the file to share.

@DissectMalware
Copy link
Owner

The underlying issue could stem from parsing errors in the parent nodes. Without access to the actual file, identifying the precise location of these errors would be challenging. Alternatively, you can enable DEBUG mode and share the output with me; this might give more context.

@cccs-jh
Copy link
Author

cccs-jh commented Feb 1, 2024

Here is the DEBUG mode log:

1044 ObjectSpaceManifestListReferenceFND 2
3092 ObjectSpaceManifestListStartFND 0
3116 RevisionManifestListReferenceFND 2
3380 RevisionManifestListStartFND 0
3408 RevisionManifestStart4FND 0
3466 GlobalIdTableStartFNDX 0
3470 Invalid 0
25140 RevisionManifestStart4FND 0
25198 GlobalIdTableStartFNDX 0
25202 Invalid 0
25206 Invalid 0
25210 Invalid 0
25214 Invalid 0
3123 Invalid 0
1071 ObjectSpaceManifestRootFND 0
1095 ObjectSpaceManifestListReferenceFND 2
3908 ObjectSpaceManifestListStartFND 0
3932 RevisionManifestListReferenceFND 2
4196 RevisionManifestListStartFND 0
4224 RevisionManifestStart4FND 0
4282 GlobalIdTableStartFNDX 0
4286 ObjectSpaceManifestRootFND 0
4310 Invalid 0
4314 Invalid 0
4318 Invalid 1
4322 Invalid 0
4326 Invalid 0
4330 Invalid 10
4334 Invalid 1
4338 Invalid 0
4342 Invalid 0
4346 Invalid 1
22212 Invalid 1
22216 Invalid 2

The first issue seems to be that GlobalIdTableStartFNDX isn't implemented. Since it has a reserved data byte that isn't being read, the following FileNodes are offset by one byte and become invalid. Fixing that (and GlobalIdTableEntry3FNDX) gives the following debug information:

1044 ObjectSpaceManifestListReferenceFND 2
3092 ObjectSpaceManifestListStartFND 0
3116 RevisionManifestListReferenceFND 2
3380 RevisionManifestListStartFND 0
3408 RevisionManifestStart4FND 0
3466 GlobalIdTableStartFNDX 0
3471 GlobalIdTableEntryFNDX 0
3495 GlobalIdTableEntryFNDX 0
3519 GlobalIdTableEndFNDX 0
3523 Invalid 1
3527 Invalid 1
3531 Invalid 6
25140 RevisionManifestStart4FND 0
25198 GlobalIdTableStartFNDX 0
25203 GlobalIdTableEntry3FNDX 0
25219 GlobalIdTableEndFNDX 0
25223 Invalid 1
25227 Invalid 1
25231 Invalid 3
3123 Invalid 0
1071 ObjectSpaceManifestRootFND 0
1095 ObjectSpaceManifestListReferenceFND 2
3908 ObjectSpaceManifestListStartFND 0
3932 RevisionManifestListReferenceFND 2
4196 RevisionManifestListStartFND 0
4224 RevisionManifestStart4FND 0
4282 GlobalIdTableStartFNDX 0
4287 GlobalIdTableEntryFNDX 0
4311 GlobalIdTableEndFNDX 0
4315 Invalid 1
4319 Invalid 1
4323 Invalid 6
22212 Invalid 1
22216 Invalid 2

Which is better but the GlobalIdTables still seem like they are followed by junk/uninterpretable data. Which they might be, the file is wrong in a couple ways, notably that it is a .one and not a .onetoc2 file per guidFileType but contains .onetoc2 specific structures. It also has a guidLegacyFileVersion set and the ffv... series of metadata are set to 0x16 instead of 0x2A. So the program that wrote the file is probably old and non-compliant with the spec.

@cccs-jh
Copy link
Author

cccs-jh commented Feb 1, 2024

I've updated the request with the changes that allow the onetoc2 GlobalIdTable to be parsed without missalignment.

@DissectMalware
Copy link
Owner

In a few days, I will check the file format reference and merge if I couldn't find an issue. Thanks for all of your efforts on this

@cccs-jh
Copy link
Author

cccs-jh commented Feb 7, 2024

You're welcome. Thank you for the interest.

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