Adding support for FileStructure exploitation #1218
Conversation
vigneshsrao
force-pushed
the
dev
branch
2 times, most recently
from
a0cb2c2
to
0215412
Compare
|
Overall this looks good, and most of the heavy lifting is done! Cool stuff! I've added some comments about convention ( If you can clean this up a bit, we can get this merged! |
pwnlib/FilePointer.py
Outdated
| var['unknown2']=48 | ||
| return var | ||
|
|
||
| def packit(s,l=8): |
There was a problem hiding this comment.
This needs documentation and tests
pwnlib/FilePointer.py
Outdated
| var[i]=null | ||
| return var | ||
|
|
||
| def update_var(l): |
There was a problem hiding this comment.
This needs documentation and tests
pwnlib/FilePointer.py
Outdated
| '_wide_data' | ||
| ] | ||
|
|
||
| def get_defaults(null): |
There was a problem hiding this comment.
Indentation is wrong here. I'd recommend looking at collections.defaultdict to avoid the need for this.
pwnlib/FilePointer.py
Outdated
| >>> fileStr.flags = 0xfbad1807 | ||
| >>> fileStr._IO_buf_base = 0xcafebabe | ||
| >>> fileStr._IO_buf_end = 0xfacef00d | ||
| >>> payload = str(fileStr) |
There was a problem hiding this comment.
This should show what the payload looks like, so that there are actual tests.
pwnlib/FilePointer.py
Outdated
|
|
||
| >>> context.clear(arch='amd64') | ||
| >>> fileStr = FileStructure(0xdeadbeef) | ||
| >>> payload = fileStr.read(addr=0xcafebabe, size=100) |
pwnlib/FilePointer.py
Outdated
| self['fileno'] = 1 | ||
| return self.struntil('fileno') | ||
|
|
||
| def read(self,addr=0,size=0): |
pwnlib/FilePointer.py
Outdated
| break | ||
| return structure[:len(structure)-1] | ||
|
|
||
| def write(self,addr=0,size=0): |
pwnlib/FilePointer.py
Outdated
| def set_vars(self,item,value): | ||
| self[item]=value | ||
|
|
||
| def struntil(self,v): |
pwnlib/FilePointer.py
Outdated
| structure='' | ||
| for val in self.vars_: | ||
| if isinstance(self[val],str): | ||
| if self.arch=='i386': |
There was a problem hiding this comment.
Do not use self.arch, use context.arch
pwnlib/FilePointer.py
Outdated
| return structure[:len(structure)-1] | ||
|
|
||
| def write(self,addr=0,size=0): | ||
| self['flags'] &=~8 |
There was a problem hiding this comment.
It looks like we're inheriting from dict in order to set these fields.
Just use regular fields, i.e. self.flags, self._IO_write_base, etc.
This way these fields can be documented.
There was a problem hiding this comment.
So should we remove the inheritance from dict altogether? Or should we just add documentation for the various fields?
There was a problem hiding this comment.
Are there any places we leverage the dict inheritance other than to set self['field'] = value?
There was a problem hiding this comment.
While setting the default data we update the dict. Also, the str and struntil functions involve checking the type of the value of each key, while iterating through the list of fields. I am not quite sure how we can iterate through the list of fields while checking the type of the value they hold if all the fields are set as independent class attributes.
|
Are the fields not enumerable before runtime? Keeping a list of fields we
care about, and using getattr() would be equivalent.
On Wed, Oct 31, 2018 at 4:43 PM Vignesh S Rao ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pwnlib/FilePointer.py
<#1218 (comment)>:
> + return ''
+ structure=''
+ for val in self.vars_:
+ if isinstance(self[val],str):
+ if self.arch=='i386':
+ structure+=self[val].ljust(4,'\x00')
+ else:
+ structure+=self[val].ljust(8,'\x00')
+ else:
+ structure+=packit(self[val],self.length[val])
+ if val==v:
+ break
+ return structure[:len(structure)-1]
+
+ def write(self,addr=0,size=0):
+ self['flags'] &=~8
While setting the default data we update the dict. Also, the str and
struntil functions involve checking the type of the value of each key,
while iterating through the list of fields. I am not quite sure how we can
iterate through the list of fields while checking the type of the value
they hold if all the fields are set as independent class attributes.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1218 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAG0GAlwtDOGADs-GTL3WJdZR8iOIl7Bks5uqhmMgaJpZM4X9lfL>
.
--
*Zach Riggle*
|
|
Neat! Did not think of this before. I'll make the necessary changes as soon as possible. Thanks a lot! |
|
Completed the necessary changes. |
|
Hi, are there any plans on getting this merged? :) |
|
@zachriggle Are there any more changes to be made to the code? |
also rename the module to lower case to keep consistency
This is in reference to #1217.
The functioning of this class is similar to srop.py. One has to create an object of this class and then assign member variables, similar to the C fashion. The python class basically emulate's the C
FILEstructure and makes it easier to write payloads for exploiting theFILEstructure. It also includes methods to generate payloads for some standard cases like arbitrary read and write. Onlyamd64andi386architectures are supported currently.The python file itself contains a full documentation. Please refer here for a detailed overview of the features.