Closed GoogleCodeExporter closed 9 years ago
Here's a patch for this. While I'm pretty positive there are no issues with it,
I'd appreciate if at least one person could verify its OK.
Original comment by michael.hale@gmail.com
on 28 Oct 2011 at 2:11
Attachments:
We shouldn't necessarily be matching the previous output unless the value
returned previously makes more sense. It depends if people are going to reuse
the value in a different plugin, or if they're more likely to compare it with
old versions. It looks like strictly the _OBJECT_HEADER is a part of the
_FILE_OBJECT, so if we're after the _FILE_OBJECT's offset, then we should
return that, not cut off the first field because that's how we've always done
it. Having said all that I haven't looked into it enough, I just want to make
sure we're not doing things blindly. This should be a considered choice...
Looking at the patch, it doesn't have any error checking in case Body isn't
valid, but that's exactly what NoneObjects are supposed to handle, so yeah,
should be fine if we decide that the change is the right thing to do.
Original comment by mike.auty@gmail.com
on 28 Oct 2011 at 6:41
Yes, definitely agree with Ikelos's point about not changing things just to be
consistent with 1.3. There are a few things that may be confusing about this,
so I'll try to be as clear as possible ;-)
First, Carl reported this issue in filescan, but my patch is for handles. At
first I thought I had made a mistake, but now it occurs to me that *if* we make
the change for filescan, then we also should make the change to handles (so
that the offset printed by handles for a given file will match the offset
printed by filescan for the same file).
Filescan is not the only plugin that carves objects with an _OBJECT_HEADER.
Mutantscan, symlinkscan, and even psscan and thrdscan are in the same boat.
However, for all plugins *except* filescan, we print the object's offset (in
other words the exact location of_KMUTANT, _OBJECT_SYMBOLIC_LINK, _EPROCESS,
and _ETHREAD respectively) rather than the offset of _OBJECT_HEADER.
So what Carl has found is an inconsistency among filescan and all the other
scanners mentioned (and in my opinion the behavior of the other scanners is
what we *should* be doing - regardless of whether that's what was done in 1.3
or not).
> It looks like strictly the _OBJECT_HEADER is a part of the _FILE_OBJECT
Yes, in memory the placement looks like:
[_POOL_HEADER] [OPTIONAL_OBJECT_HEADER_FIELDS] [_OBJECT_HEADER] [_FILE_OBJECT]
^ ^
points now should point
> Looking at the patch, it doesn't have any error checking in case Body isn't
valid
Hmm, well Body isn't a pointer, its just a member of the _OBJECT_HEADER (0x18
bytes from the base). So unless we hit Issue #57 (objects that cross page
boundaries) then it should be safe to assume that if _OBJECT_HEADER is valid,
then _OBJECT_HEADER.Body is valid. Right?
The attached patch should account for changing the printed offset for filescan
and handles.
Original comment by michael.hale@gmail.com
on 28 Oct 2011 at 2:09
Attachments:
Just realized my cool ascii memory layout isn't so cool when viewed on the
page...sorry...you can probably guess what its supposed to look like.
Original comment by michael.hale@gmail.com
on 28 Oct 2011 at 2:10
heh, it looks ok in the email notification, just formatted differently here.
Original comment by jamie.l...@gmail.com
on 28 Oct 2011 at 2:14
I've tested file extraction (via my exportfile.py and the Honeynet challenge
Bob.vmem sample), using both Michael's handle_offset.patch and my original
"bodge" patch above.
Comparing extracted directory structures with sha1deep shows consistent outputs
between the two patches.
So, from my point of view, Michael's patch fixes this issue.
Original comment by carl.pulley
on 1 Nov 2011 at 9:47
Thanks for testing Carl. Unless someone has objections (ikelos, scudette?) I'll
apply the patch early next week.
Original comment by michael.hale@gmail.com
on 13 Nov 2011 at 6:37
Looks ok to me, no objections here.
Original comment by mike.auty@gmail.com
on 13 Nov 2011 at 6:47
This issue was closed by revision r1145.
Original comment by michael.hale@gmail.com
on 14 Nov 2011 at 6:02
Original issue reported on code.google.com by
carl.pulley
on 9 Oct 2011 at 1:37