feat: add initial support for size_t#232
Conversation
alexkornitzer
force-pushed
the
master
branch
from
7044f50 to
8f33d01
Compare
alexkornitzer
changed the title
ohadravid
left a comment
ohadravid
left a comment
There was a problem hiding this comment.
Hi!
Thanks for this PR 💪
Do you think you could share the sample publicly?
| } | ||
| (BinXmlValueType::SizeTType, Some(8)) => { | ||
| BinXmlValue::HexInt64Type(try_read!(cursor, hex64)?) | ||
| } |
There was a problem hiding this comment.
Maybe also keep the old error in case it's not 4/8?
There was a problem hiding this comment.
Happy to do whatever the maintainers would like for this as its your domain. The reason I did not keep the old error is that I thought the catch all at the bottom would be sufficient now, as it would be there for edge cases? But I guess if we do that the error type is now redundant and needs to be removed?
There was a problem hiding this comment.
It will catch it, but keeping the special match arm gives a better error so it's a bit nicer (as we already have an error just for that)
|
Yep here is the sample, which was taken from here, challenge 1 (https://www.ashemery.com/dfir.html) |
|
Great! So could you add a test like |
From looking at an event log which utilises size_t it can be seen to output size_t in hex. This commit adds support for widths 4 & 8, by applying them to the HexInt32 and HexInt64 variants.
alexkornitzer
force-pushed
the
master
branch
from
8f33d01 to
e6ac3a8
Compare
|
Done and squashed to keep the commit log clean. |
3 participants
Heya,
Very happy to finally have something to provide upstream to such a core crate in Chainsaw :)
Someone at work provided me with an evtx file which was failing to parse due to this size_t type.
From looking at an event log which utilises size_t it can be seen to output size_t in hex. This commit adds support for widths 4 & 8, by applying them to the HexInt32 and HexInt64 variants.
HandleId & ProcessId:
The assumptions above would make sense to me as this is usually how size_t is typedef'd. But I fully get if you would not want to merge this as it has only been tested on one sample and some assumptions have been made.