API / convenience functions for handles

[GoogleCodeExporter]

This is just one of many API-ish suggestions I'll make over time, just to 
generate discussion and see if others think its useful. Unlike the recent VAD 
issue, I have not already developed a patch for this, so its open for anyone ;-)

What I'd like to propose is:

1) the addition of handle values to the output of "handles" command. the handle 
value is essentially just an index into the handle table

2) an API that returns the OBJECT_HEADER for a given handle value. for example:

objhdr = proc.ObjectTable.get_handle(0x24) 

This is important, not only for plugin writers, but for practical analysis 
purposes. Some structures store a handle value such as 0x24 and we need to be 
able to convert that into the corresponding object. 

Original issue reported on code.google.com by michael.hale@gmail.com on 17 Aug 2011 at 5:03

[GoogleCodeExporter]

Here's a patch based on r1057 that adds the handle value and granted access to 
the output. The biggest difference is how _EPROCESS.ObjectTable.handles() 
operates.

Instead of this:

for object in proc.ObjectTable.handles()

Now we can do:

for hval, access, object in proc.ObjectTable.handles()

Where hval is the handle value, access is the GrantedAccess (_ACCESS_MASK), and 
object is the same as before - an _OBJECT_HEADER. 

Sample output:

Volatile Systems Volatility Framework 2.1_alpha
Offset(V)    Pid    Handle     Access     Type             Details
0x83940970   4      0x4        0x1fffff   Process          System(4)
0x8760bf30   4      0x8        0x0        Key              
0x8760b270   4      0xc        0x2001f    Key              
MACHINE\SYSTEM\CONTROLSET001\CONTROL\HIVELIST
0x8760b088   4      0x10       0x20019    Key              
MACHINE\HARDWARE\DESCRIPTION\SYSTEM\MULTIFUNCTIONADAPTER
0x8765f890   4      0x14       0xf003f    Key              
MACHINE\SYSTEM\CONTROLSET001\CONTROL\SESSION MANAGER\MEMORY 
MANAGEMENT\PREFETCHPARAMETERS
0x87664748   4      0x18       0x2001f    Key              
MACHINE\SYSTEM\CONTROLSET001\CONTROL\PRODUCTOPTIONS
0x83966430   4      0x1c       0x1f0001   ALPC Port        'PowerMonitorPort'
0x83966840   4      0x20       0x1f0001   ALPC Port        'PowerPort'

If you wanted to check if a given handle (for example 0x410) is in use for a 
process, you have a few choices. 

First you could do:

for hval, access, object in proc.ObjectTable.handles():
    if hval == 0x410:
        # do something

Or if you plan to do several checks of different handles, and don't want to 
iterate through them more than once, just read them into a dictionary, like 
this:

handles = dict( (hval,(access,object)) for (hval,access,object) in 
proc.ObjectTable.handles())

Now you can perform checks like handles.has_key(0x410), handles.get(0x410, 
None), or just handles[0x410] inside a try/except KeyError block. 

I had a little trouble accessing the handle table level/depth inside the 
recursive function. Can someone take a look at how I'm calculating the handle 
value and see if there's a better way to do it?

Questions, comments, thoughts, complaints???

Thanks!

Original comment by michael.hale@gmail.com on 18 Aug 2011 at 3:37

Attachments:

• handles_patch.txt

[GoogleCodeExporter]

Hiya Michael, 

It's early in the morning for me, so I apologize if any of my comments are 
meaningless/daft.  I'm wondering why we can't just name the handles once they 
come out out the make_array function?  If they come out of that in order, and 
they're always the same size, surely the index they come out of that will be 
enough to determined the position in the table (unless there are gaps, etc?).  
I haven't checked out the code closely in a while, so I'd need to have a think 
through and check this is right, but if anyone else knows off the top of their 
head, this might also be a good way of getting around my second point.

I'd prefer not to change/break the API unless absolutely necessary, so I'd 
rather use a different function than handles (a new one), or add an optional 
(but usually off) parameter that specifies whether the output should be short 
or long.  Ideally all this could be done outside of the function itself (which 
is why I'm hoping the ordering can be done based on the existing results).

I'll look at it again once I have more time/working brain cells though...  5:)

Original comment by mike.auty@gmail.com on 18 Aug 2011 at 6:44

[GoogleCodeExporter]

If I can make a suggestion to attach the hval and access as members of the 
object rather than yield them separately. This will not change the API and also 
make it simpler to use as if you dont particularly care about these you can 
just ignore them.

Original comment by scude...@gmail.com on 18 Aug 2011 at 7:09

[GoogleCodeExporter]

Yep, that would be another solution, although I'm not so keen on tacking 
arbitrary attributes on to existing objects because people will always have to 
error check in case they ever manually instantiate the handle type.  Also 
making new objects that don't behave exactly the same way breaks the API, 
(which is where adding attributes to the existing object would be a better 
solution).

Another option may be to add a helper function that given an object can return 
you the values you're interested in, so something like 
ObjectTable.get_handle_value(handle), which would then either re-traverse or 
have kept a cache or something (assuming we're not on a live AS)?

Original comment by mike.auty@gmail.com on 18 Aug 2011 at 7:18

[GoogleCodeExporter]

Hey guys, 

Nothing is meaningless/daft. Your input is what keeps this project clean and 
mediated.  

> I'm wondering why we can't just name the handles once they come 
> out out the make_array function? (unless there are gaps, etc?)

make_handle_array only yields valid / used indexes in the handle table...so 
there will always be gaps. 

> If I can make a suggestion to attach the hval and access as members of the 
> object rather than yield them separately. 

Hmm so the object is _OBJECT_HEADER, but the GrantedAccess is actually a member 
of _HANDLE_TABLE_ENTRY. A _HANDLE_TABLE_ENTRY points to the _OBJECT_HEADER, but 
not the other way around. So theoretically (not saying we should do it) if 
make_handle_array yielded the _HANDLE_TABLE_ENTRY instead of _OBJECT_HEADER 
then it would be pretty simple to associate, but since we only yield 
_OBJECT_HEADER, its not so easy. Your "attach" method sounds interesting 
(although Ikelos has some disadvantages stated). 

If make_handle_array sets the parent for _OBJECT_HEADER when its created, would 
that work? For example:

object = obj.Object("_OBJECT_HEADER", offset, vm, parent = handle_table_entry)

Then we wouldn't need to yield the access since it can be referenced by 
object.obj_parent.GrantedAccess. Of course, we'd still need a solution for 
attaching the handle value. 

I think this is an interesting problem because to compute the handle value, you 
need 3 pieces of info:

1) The offset of _HANDLE_TABLE_ENTRY
2) The base of the table containing the _HANDLE_TABLE_ENTRY 
3) The level / depth (i.e. number of tables already traversed to reach #2)

The make_handle_array function is the only place where all 3 fields are 
accessible. So without changing what make_handle_array yields/returns, it is 
difficult for me to figure out how to get to the data I need. 

As always, thanks for your design remarks and I look forward to figuring the 
best way to do this...

Original comment by michael.hale@gmail.com on 18 Aug 2011 at 3:01

[GoogleCodeExporter]

I dont know if this works, but does changing _HANDLE_TABLE.get_item() to pass 
as self, the table entry help? I mean that way for each _OBJECT_HEADER yielded, 
the _HANDLE_TABLE_ENTRY will be its parent, and the table itself is its grand 
parent.

I dont see any problem with passing level/depth directly in the Array instances 
since they are temporary anyway.

Mike, I dont see a problem with attaching these attributes to the object (they 
can default to None or something). Clearly if you just instantiate the 
_OBJECT_HEADER you wont know anything about the handle table so these can 
default to None. But when using the handles() function, these values can be 
populated. The API for each object can be customised to whatever makes sense 
for it through the overlay (which is basically how the handles() method is 
tacked on).  In this case it is a convenience API to collect relevant 
information together which seems fine to me.  When instantiating the object 
with no context, this extra information is not known and wont be available - 
which is fine from an API perspective. I would also like to maintain the API as 
it is (especially the number of returned parameters) if possible.

Original comment by scude...@gmail.com on 18 Aug 2011 at 3:24

[GoogleCodeExporter]

Here's a patch using the parent / grand-parent idea, which makes it easy to 
find GrantedAccess without changing the API. The make_handle_array in this 
patch calculates handle value, but doesn't attach it yet. Can I get some 
suggestions on how best to perform the attach? 

Original comment by michael.hale@gmail.com on 18 Aug 2011 at 4:20

Attachments:

• handles_patch2.txt

[GoogleCodeExporter]

Hiya,

So my problem is someone writing code that tries to access 
_OBJECT_HEADER.handle_value, and getting a "never heard of handle_value 
attribute for normal _OBJECT_HEADER types" exception.  It's rare, but if we 
make it look like you can just access it whenever, then people may start doing 
that.

I would suggest a plugin-local overlay, but since this is a core function we're 
talking about, making the change to the object would pollute every plugin.  
That might be something we're willing to do, but I'd like to look for 
alternatives first, if anyone can think of any?  My concern at that point will 
be that people treat it as an accepted method of handling problems and start 
annotating objects with whatever they feel like.  That's fine on a per-plugin 
basis, but less good on a volatility-wide basis...

Given we can now do parental access to get back to the main parent, could we 
not use that to calculate the level (how far through parent is entry, how far 
through table is parent, multiply, add, result)?  Possibly the calculating 
plugin could cache the "parent in table" locations?  I realize that's pushing 
knowledge of the structures out of the convenience functions, and it might be 
slightly inefficient, but I'd class it as much, much cleaner.  Depends how slow 
it works out?

So, I'm kind of ok with it, but would prefer to explore other 
alternatives/suggestions before saying yes...

Original comment by mike.auty@gmail.com on 18 Aug 2011 at 5:59

[GoogleCodeExporter]

Mike,
   I dont see this as an issue. If we change the _OBJECT_HEADER overlay to initialize self.handle_value = NoneObject("Unknown") then there is no problem. If people access it from a fresh instance it will fail with an Unknown message which is exactly the right behaviour.

I feel that the recursion through the parents is a bit messy and feels a bit 
like a clumsy API. It would make most sense to put that recursion in a function 
attached to the _OBJECT_HEADER itself (e.g. _OBJECT_HEADER.get_handle_value()) 
but that will have the same problems if the parents are not initialized 
correctly.

It seems better to me to encapsulate all information about an object in object 
attributes rather than utility functions that keep having to derive these 
external to the object.

Original comment by scude...@gmail.com on 18 Aug 2011 at 6:51

[GoogleCodeExporter]

Per dev call, instead of using a parent, create a volmagic for GrantedAccess in 
xpsp2 overlay (so its carried through to all others), then set GrantedAccess 
for the handles plugin

Original comment by michael.hale@gmail.com on 15 Dec 2011 at 7:55

[GoogleCodeExporter]

Scudette, can you give this patch a look? Let me know if its what you 
wanted/expected.

After creating the overlay, I ran into an issue with assignment through '=' and 
setattr due to write() support not being enabled in the AS:

object_header.GrantedAccess = handle_table_entry.GrantedAccess
setattr(object_header, 'GrantedAccess', handle_table_entry.GrantedAccess)

So instead I did this:

object_header.newattr('GrantedAccess', handle_table_entry.GrantedAccess)

That seems to work, but seems strange that I'm using newattr when GrantedAccess 
should already be an attribute through the overlay. 

Original comment by michael.hale@gmail.com on 15 Dec 2011 at 10:36

• Changed state: Started

Attachments:

• issue_135.patch

[GoogleCodeExporter]

That because assigning to a member is how you write stuff to a memory image.  
The fact that GrantedAccess is more of a fictitious member, rather than a real 
one, makes the whole thing kind of difficult.  A better way would be to make it 
a Class that overrides the _OBJECT_HEADER type, and includes its own property 
based on a setter/getter (or possibly just a self.blah thing).  One way or 
another the object will allow you to store and recover local state... 

Original comment by mike.auty@gmail.com on 15 Dec 2011 at 11:15

[GoogleCodeExporter]

You could always just add it to the object's members dict:

object_header.members["GrantedAccess"] = handle_table_entry.GrantedAccess

Original comment by scude...@gmail.com on 15 Dec 2011 at 11:25

[GoogleCodeExporter]

Ah so many options ;-) 

Thanks guys. Is the below patch more appropriate? 

Index: volatility/plugins/overlays/windows/windows.py
===================================================================
--- volatility/plugins/overlays/windows/windows.py  (revision 1159)
+++ volatility/plugins/overlays/windows/windows.py  (working copy)
@@ -266,6 +266,10 @@
                     if item == None:
                         continue

+                    # carry over the access from _HANDLE_TABLE_ENTRY and make 
it  
+                    # an "fake" member of _OBJECT_HEADER. see Issue 135. 
+                    item.members['GrantedAccess'] = entry.GrantedAccess
+
                     try:
                         # New object header
                         if item.TypeIndex != 0x0:
Index: volatility/plugins/handles.py
===================================================================
--- volatility/plugins/handles.py   (revision 1159)
+++ volatility/plugins/handles.py   (working copy)
@@ -55,8 +55,8 @@
     def render_text(self, outfd, data):
         offsettype = "(V)" if not self._config.PHYSICAL_OFFSET else "(P)"

-        outfd.write("{0:6}{1:6} {2:6} {3:<16} {4}\n".format(
-            "Offset", offsettype, "Pid", "Type", "Details"))
+        outfd.write("{0:6}{1:6} {2:6} {3:10} {4:<16} {5}\n".format(
+            "Offset", offsettype, "Pid", "Access", "Type", "Details"))

         if self._config.OBJECT_TYPE:
             object_list = [s for s in self._config.OBJECT_TYPE.split(',')]
@@ -74,8 +74,8 @@
             else:
                 offset = h.obj_vm.vtop(h.Body.obj_offset)

-            outfd.write("{0:#010x}   {1:<6} {2:<16} {3}\n".format(
-                offset, pid, otype, name))
+            outfd.write("{0:#010x}   {1:<6} {2:<#10x} {3:<16} {4}\n".format(
+                offset, pid, h.members['GrantedAccess'], otype, name))

     def calculate(self):
         ## Will need the kernel AS for later:

Original comment by michael.hale@gmail.com on 15 Dec 2011 at 11:43

[GoogleCodeExporter]

LGTM

Original comment by scude...@gmail.com on 16 Dec 2011 at 12:25

[GoogleCodeExporter]

Directly accessing members is ugly, since it's hacking at the internal state of 
the objects.  members to us is like __dict__ to python, and whilst you can 
access it, it's discouraged, so consider yourself discouraged...  5;P

Could you please try mocking up the Object approach (or let me know and I can 
give it a go if you don't have time)?  I think that's much cleaner and in 
fitting with the rest of our codebase...

Original comment by mike.auty@gmail.com on 16 Dec 2011 at 8:23

[GoogleCodeExporter]

Yeah I can try the Object approach (though if you know exactly how you want it 
done, it may be easier/quicker for you to do it). 

Based on your description of the method you desire, I'm guessing it involves 
something like this:

class _OBJECT_HEADER(windows._OBJECT_HEADER):
    def __init__(self, *args, **kwargs):
        self.GrantedAccess = 0
        windows._OBJECT_HEADER.__init__(self, *args, **kwargs)

profile_object_classes["_OBJECT_HEADER"] = _OBJECT_HEADER

The win7sp0 profile from x64 branch overrides object header like this in order 
to separate a few of the methods from xp and others: 
http://code.google.com/p/volatility/source/browse/branches/win64-support/volatil
ity/plugins/overlays/windows/win7_sp0_x86.py

So if we override _OBJECT_HEADER for GrantedAccess, it seems like we'll be 
doing a lot of overriding. I was wondering if we could just add 
self.GrantedAccess to the "real" object header as we've done with self.kas - 
then it would take effect for all profiles?

class _OBJECT_HEADER(obj.CType):
    """A Volatility object to handle Windows object headers.

    This object applies only to versions below windows 7.
    """

    def __init__(self, *args, **kwargs):
        # kernel AS for dereferencing pointers 
        self.kas = None
        # this is just an example of what I mean
        self.GrantedAccess = 0
        obj.CType.__init__(self, *args, **kwargs)

I guess the downside is that if self.kas shouldn't be there anyway (and is 
likely going to be removed with the upcoming unicode/parse_string/virtual_v() 
changes) then it wouldn't make sense to add self.GrantedAccess as done above. 

So I can put together a few patches using different methods, but if you have a 
specific method in mind, it might save us both a little time. There's a 2nd 
part to this issue 135 (printing the handle value) which will require something 
similar, so perhaps if you show me your desired method of adding GrantedAccess 
I can then follow by example and add the handle value code. 

Original comment by michael.hale@gmail.com on 16 Dec 2011 at 5:49

[GoogleCodeExporter]

Ideally we'd override _OBJECT_HEADER(obj.CType) as we've done with self.kas, 
but instead provide a function that can calculate the GrantedAccess value 
through the parent to determine it.  If that turns out to be completely 
unfeasible, then I'd go with doing self.GrantedAccess = 0 and caching the 
value, but it would be far better to calculate the value live each time.

Original comment by mike.auty@gmail.com on 17 Dec 2011 at 12:36

[GoogleCodeExporter]

Alright, I will draft up a patch to use the parent. I thought we had decided 
against that on the dev call, but it won't hurt to have a few options ready 
before making a final decision. Thanks Ikelos!

Original comment by michael.hale@gmail.com on 20 Dec 2011 at 3:27

[GoogleCodeExporter]

I must've missed that, but if there's good reasoning behind it, then that's 
fine.

My reasoning for doing it that way is that having two different values which 
hold the same thing is bad because they could get out of sync and cause 
problems, so if it's easy to calculate then we should.  If it's in a 
convenience function on the object then I can't see the problem, but I may have 
missed something, so do let me know...  5:)

Original comment by mike.auty@gmail.com on 20 Dec 2011 at 10:22

[GoogleCodeExporter]

Hmm actually I submitted a patch back in August (see Comment #7) above that 
used the parent method to associate the _HANDLE_TABLE_ENTRY.GrantedAccess with 
the _OBJECT_HEADER. I guess then we would need to do something like this:

class _OBJECT_HEADER(obj.CType):

    def get_granted_access(self):
        if self.parent != None and hasattr(self.parent, 'GrantedAccess'):
            return self.parent.GrantedAccess
        return None

Is that what you had in mind? The problem I see is that there's another value I 
wanted to print in the handles output - the handle value. The value is 
calculated inside _HANDLE_TABLE._make_handle_array (see also the patch from 
Comment #7), and its derived from the _HANDLE_TABLE_ENTRY offset and the 
current table depth. In other words we couldn't use the parent method to track 
handle value as we do for GrantedAccess, unless we "attach" the handle value 
some other way (like _HANDLE_TABLE_ENTRY.members['HandleValue'] = 
TheHandleValue) except that's discouraged. 

So what other options do we have?

Original comment by michael.hale@gmail.com on 24 Dec 2011 at 6:25

[GoogleCodeExporter]

I honestly think we are complicating this too much. Lets just use what
works until we can prove its a bad idea. I dont see any problems with
manipulating the members array directly, although this probably should
be done by the OBJECT_HEADER class itself (rather than by whatever is
instantiating it).

What about something like this:

class _OBJECT_HEADER(windows._OBJECT_HEADER):
    def __init__(self, *args, **kwargs):
        # Do this
        self.GrantedAccess = kwargs.get("granted_access", 0)

        # or this
        self.members["GrantedAccess"] = kwargs.get("granted_access", 0)
        windows._OBJECT_HEADER.__init__(self, *args, **kwargs)

Then just pass it to the constructor in handles.py.

I dont like to have special methods like obj.get_XXX() - instead we
should make these as virtual attributes obj.GrantedAccess which your
original patch did fine.

Michael.

Original comment by scude...@gmail.com on 24 Dec 2011 at 12:23

[GoogleCodeExporter]

I'm still going to strongly recommend that we calculate these from their 
accessible value whenever they're required.  We don't pre-populate a 
WinTimeStamp's windows_to_unix_time value, we have a function for it.  Why 
should we consider not doing it here?  Just because the value resides in a 
linked object that we can easily access?

Caching this stuff will end up creating subtle bugs that'll be a pain to track 
down for the benefit of some slight syntactic sugar.  Leaving ourselves bugs 
because we don't want to spend the effort fixing them in the design stage is 
not a good attitude.

Michael, you say you're not a fan of get_XXX() functions, and yet there's 
already get_process_address_space, get_init_modules, get_mem_modules, 
get_load_modules and get_item in the codebase to name just a few within 100 
lines of the code we're looking at.

If it's going to be ridiculously inefficient to calculate the HandleValue 
whenever it's required, and we absolutely must tag data into existing objects, 
the best way of handling it is as Michael pointed out:

class _OBJECT_HEADER(obj.CType):
    def __init__(self, *args, **kwargs):
        self.HandleValue = kwargs.get("handle_value", 0)

Original comment by mike.auty@gmail.com on 24 Dec 2011 at 12:45

[GoogleCodeExporter]

Hey guys, 

Merry xmas! 

> If it's going to be ridiculously inefficient to calculate the HandleValue 
whenever it's required, 
> and we absolutely must tag data into existing objects, 

I don't see a way around it unfortunately. Calculating the handle value 
requires using numbers that *only* exist inside 
_HANDLE_TABLE._make_handle_array. So either we yield the necessary values with 
the object (that changes the API) or we compute the handle value in 
_make_handle_array and attach/tag it to the object. 

So although get_XXX() would work for GrantedAccess, it wouldn't work for 
HandleValue...unless get_handle_value did something like return 
self.HandleValue - but that doesn't save us from attaching/tagging in the first 
place. The kwargs suggestion is acceptable to me if you guys are OK with it. Of 
course, it sounded easy but when I tried it, I ran into a problem. 

For example, using the following patch (on the win64 branch):

===================================================================
--- volatility/plugins/overlays/windows/windows.py  (revision 1162)
+++ volatility/plugins/overlays/windows/windows.py  (working copy)
@@ -221,12 +221,17 @@
     tables, such as the _KDDEBUGGER_DATA64.PspCidTable.
     """

-    def get_item(self, offset):
+    def get_item(self, entry, handle_value):
         """Returns the OBJECT_HEADER of the associated handle at a particular offset"""
-        return obj.Object("_OBJECT_HEADER", offset, self.obj_vm,
-                                            parent = self)

-    def _make_handle_array(self, offset, level):
+        return obj.Object("_OBJECT_HEADER", 
+                          offset = entry.Object.v() & ~7, # an _EX_FAST_REF
+                          vm = self.obj_vm, 
+                          parent = self, 
+                          granted_access = entry.GrantedAccess, 
+                          handle_value = handle_value)
+
+    def _make_handle_array(self, offset, level, depth = 0):
         """ Returns an array of _HANDLE_TABLE_ENTRY rooted at offset,
         and iterates over them.

@@ -248,15 +253,18 @@

                 if level > 0:
                     ## We need to go deeper:
-                    for h in self._make_handle_array(entry, level - 1):
+                    for h in self._make_handle_array(entry, level - 1, depth):
                         yield h
+                    depth += 1
                 else:
                     ## OK We got to the bottom table, we just resolve
                     ## objects here:
-                    obj_offset = int(entry.Object.v()) & ~0x00000007

-                    item = self.get_item(obj_offset)
+                    ## calculate the handle value, see Issue 135. 
+                    handle_value = ((entry.obj_offset - offset) / 4) + (depth 
* 0x800)

+                    item = self.get_item(entry, handle_value)
+
                     if item == None:
                         continue

@@ -307,6 +315,10 @@
     def __init__(self, *args, **kwargs):
         # kernel AS for dereferencing pointers 
         self.kas = None
+
+        self.GrantedAccess = kwargs.get("granted_access", 0)
+        self.HandleValue = kwargs.get("handle_value", 0)
+
         obj.CType.__init__(self, *args, **kwargs)

The GrantedAccess and HandeValue show up all zero:

$ python vol.py --profile=Win7SP1x64 -f ~/Desktop/memory.dd handles
Volatile Systems Volatility Framework 2.1_alpha
Offset(V)    Pid    Handle     Access     Type             Details
0xfffffa8001e37ef0   268    0x0        0x0        File             \Windows
0xfffffa8001e5af80   268    0x0        0x0        EtwRegistration  -
0xfffffa8001e48540   268    0x0        0x0        File             
\ProtectedPrefix
0xfffffa8001e483f0   268    0x0        0x0        File             
\ProtectedPrefix
0xfffffa8001e482a0   268    0x0        0x0        File             
\ProtectedPrefix\Administrators
0xfffffa8001e4bef0   268    0x0        0x0        File             
\ProtectedPrefix\Administrators

I traced the problem back to obj.py in the Object class:

    try:
        if theType in vm.profile.types:
            result = vm.profile.types[theType](offset = offset, vm = vm, name = name,
                                               parent = parent)
            return result

        if theType in vm.profile.object_classes:
            result = vm.profile.object_classes[theType](theType = theType,
                                                        offset = offset,
                                                        vm = vm,
                                                        parent = parent,
                                                        name = name,
                                                        **kwargs)
            return result

Since _OBJECT_HEADER is in vm.profile.types, it gets instantiated without the 
**kwargs. So the lines kwargs.get("granted_access", 0) always is 0. Any 
thoughts on how to fix that? 

Original comment by michael.hale@gmail.com on 28 Dec 2011 at 2:58

[GoogleCodeExporter]

Hiya Michael,

So in the patches I sent round a few days ago one of the first things I cleaned 
up in the object model so that parameters should always get passed through to 
the lower layers.  In short, you can just add **kwargs to the end of that bit.

Also note, that since the members are now internal members, you can simply set 
them without write support bothering you.  So:

item.HandleValue = handle_value

Also, I'd still prefer we use the get_granted_access or a property instead of 
caching, so something like:

        return obj.Object("_OBJECT_HEADER", 
                          offset = entry.Object.v() & ~7, # an _EX_FAST_REF
                          vm = self.obj_vm, 
                          parent = entry, 
                          handle_value = handle_value)

class _OBJECT_HEADER(obj.CType):

    @property
    def GrantedAccess(self):
        return self.parent.GrantedAccess

Strictly, the parent ought to be the Object pointer I guess, but I'd need to 
check our current semantics with pointers parents before I could say.

Original comment by mike.auty@gmail.com on 28 Dec 2011 at 9:55

[GoogleCodeExporter]

You guys realize that we can simply use an _ before any function to
make it a struct member:
 class _OBJECT_HEADER(obj.CType):

    def _GrantedAccess(self, attr):
        if self.parent: return self.parent.GrantedAccess
        return obj.NoneObject("No parent known")

parent may be None if not provided to the object constructor.

Michael.

Original comment by scude...@gmail.com on 28 Dec 2011 at 10:49

[GoogleCodeExporter]

Sure, I'm just not sure how implementing our own property system is 
better/less-confusing than using python's built-in property system?

Definitely good to have a check on parent, though...

Original comment by mike.auty@gmail.com on 28 Dec 2011 at 1:27

[GoogleCodeExporter]

It probably does not matter much - but thats the way all the other
member overrides are done currently. If we choose to make them all
properties, lets make it all consistent then and remove this code
(obj.py):

     def __getattribute__(self, attr):
        try:
            return object.__getattribute__(self, attr)
        except AttributeError:
            pass

        try:
            return object.__getattribute__(self, "_" + attr)(attr)
        except Exception:
            pass

        return self.m(attr)

and put this instead:
     def __getattr__(self, attr):
        return self.m(attr)

Then we can fix up all the objects in windows.py and linux_common.py
to use properties instead.

Michael.

Original comment by scude...@gmail.com on 28 Dec 2011 at 2:26

[GoogleCodeExporter]

That sounds like a good plan.  I'll try and mock up a patch in the next few 
days.

We'll need to keep the following though:

        try:
            return object.__getattribute__(self, attr)
        except AttributeError:
            pass

otherwise all standard lookups will fail (including, I suspect, those for the 
properties we're interested in!).  5:)

Original comment by mike.auty@gmail.com on 28 Dec 2011 at 2:32

[GoogleCodeExporter]

The difference between __getattribute__ and __getattr__ is that the
later happens after all standard lookups are done (if they all fail) -
simplifying the implementation greatly. I cant remember if its a new
python feature since we implemented this.

In fact we should remove all __getattribute__ references because you
rarely need to do override standard lookups.

Michael.

Original comment by scude...@gmail.com on 28 Dec 2011 at 2:36

[GoogleCodeExporter]

Seems we're one of those rare instances, this breaks when using __getattr__, 
but not when using __getattribute__ with the try/except on AttributeError.  
Anyway, here's a patch if you want to play around and try and get that working.

Original comment by mike.auty@gmail.com on 28 Dec 2011 at 6:38

Attachments:

• volatility-remove-unnecessary-getattribute-hook.patch

[GoogleCodeExporter]

Please see r1204 for the GrantedAccess patch, if its not what you 
expected/wanted please let me know. 

The other half (HandleValue) will be applied separately.

Original comment by michael.hale@gmail.com on 31 Dec 2011 at 6:54

[GoogleCodeExporter]

Ok, turns out the __getattribute__ issue seems to have been caused by mixing 
__getattr__ and __getattribute__.  I've attached a new patch that may provide 
some minor speed improvements, it's undergone a few basic tests, but it could 
really do with stronger testing...

Original comment by mike.auty@gmail.com on 22 Jan 2012 at 8:15

Attachments:

• volatility-getattr.patch

[GoogleCodeExporter]

I don't fully understand what this is doing, but I did test to make sure 
GrantedAccess still works in the handles output ;-)

Original comment by michael.hale@gmail.com on 23 Jan 2012 at 2:51

[GoogleCodeExporter]

Hehehe, thanks.  It basically lets python handle things an object declares, and 
only bothers us to check for members when it can't find them.

Could you please run a few of the major plugins (handles, vadinfo, etc) against 
a few unusual profiles, and see if you get any exceptions or changes in the 
output?

Original comment by mike.auty@gmail.com on 23 Jan 2012 at 2:54

[GoogleCodeExporter]

No changes that I can tell so far. 

However, I was looking at finishing the other half of this issue (adding 
HandleValue to handles output) and noticed that putting self.HandleValue = 
kwargs.get("handle_value", 0) in the _OBJECT_HEADER.__init__ function doesn't 
seem to work anymore (both before and after the patch).

It shows this error:

Traceback (most recent call last):
  File "vol.py", line 135, in <module>
    main()
  File "vol.py", line 126, in main
    command.execute()
  File "/Users/Michael/volatility_trunk/volatility/commands.py", line 101, in execute
    func(outfd, data)
  File "/Users/Michael/volatility_trunk/volatility/plugins/handles.py", line 51, in render_text
    for pid, handle, object_type, name in data:
  File "/Users/Michael/volatility_trunk/volatility/plugins/handles.py", line 70, in calculate
    for handle in task.ObjectTable.handles():
  File "/Users/Michael/volatility_trunk/volatility/plugins/overlays/windows/windows.py", line 423, in handles
    for h in self._make_handle_array(offset, table_levels):
  File "/Users/Michael/volatility_trunk/volatility/plugins/overlays/windows/windows.py", line 382, in _make_handle_array
    for h in self._make_handle_array(entry, level - 1, depth):
  File "/Users/Michael/volatility_trunk/volatility/plugins/overlays/windows/windows.py", line 391, in _make_handle_array
    item = self.get_item(entry, handle_value)
  File "/Users/Michael/volatility_trunk/volatility/plugins/overlays/windows/windows.py", line 355, in get_item
    return entry.Object.dereference_as("_OBJECT_HEADER", parent = entry, handle_value = handle_value)
  File "/Users/Michael/volatility_trunk/volatility/plugins/overlays/windows/windows.py", line 611, in dereference_as
    return obj.Object(theType, self.Object.v() & ~7, self.obj_native_vm, parent = parent or self, **kwargs)
  File "/Users/Michael/volatility_trunk/volatility/obj.py", line 247, in Object
    result = vm.profile.types[theType](offset = offset, vm = vm, name = name, **kwargs)
  File "/Users/Michael/volatility_trunk/volatility/plugins/overlays/windows/windows.py", line 440, in __init__
    self.HandleValue = kwargs.get("handle_value", 0)
  File "/Users/Michael/volatility_trunk/volatility/obj.py", line 787, in __setattr__
    obj = self.m(attr)
  File "/Users/Michael/volatility_trunk/volatility/obj.py", line 762, in m
    raise AttributeError("Struct {0} has no member {1}".format(self.obj_name, attr))
AttributeError: Struct _OBJECT_HEADER has no member HandleValue

Did something change recently or maybe I'm just implementing it incorrectly?

Original comment by michael.hale@gmail.com on 23 Jan 2012 at 3:36

[GoogleCodeExporter]

Ok, so first off the patch looks good, thanks for testing that.  5:)

Secondly, my guess would be it's not being implemented correctly.  If you try 
to write to an attribute after it the CType.__init__ has been called, it'll end 
up doing an attribute lookup.  I'd need to see the code you were trying first 
to be able to figure out whether that's the problem or not...

Original comment by mike.auty@gmail.com on 23 Jan 2012 at 3:39

[GoogleCodeExporter]

Ah that's it...I was doing it after CType.__init__ rather than before. Let me 
finish testing and I'll send a patch, hopefully we can finally close this 
issue. 

Original comment by michael.hale@gmail.com on 23 Jan 2012 at 4:05

[GoogleCodeExporter]

OK, here's a patch to add handle value to the handles output. I've tested that 
it prints the right values on both x86 and x64, for processes with single and 
multiple table levels. If you read through the issue, there's multiple ways of 
doing it, but the kwargs sounds like the best option. 

Original comment by michael.hale@gmail.com on 23 Jan 2012 at 5:03

Attachments:

• volatility-handle-value.patch

[GoogleCodeExporter]

This issue was closed by revision r1299.

Original comment by michael.hale@gmail.com on 23 Jan 2012 at 3:34

• Changed state: Fixed