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

Adding support for FileStructure exploitation #1218

Merged
merged 4 commits into from Dec 9, 2019
Merged

Conversation

@vigneshsrao
Copy link
Contributor

vigneshsrao commented Oct 27, 2018

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 FILE structure and makes it easier to write payloads for exploiting the FILE structure. It also includes methods to generate payloads for some standard cases like arbitrary read and write. Only amd64 and i386 architectures are supported currently.

The python file itself contains a full documentation. Please refer here for a detailed overview of the features.

@vigneshsrao vigneshsrao force-pushed the vigneshsrao:dev branch 2 times, most recently from a0cb2c2 to 0215412 Oct 27, 2018
@zachriggle

This comment has been minimized.

Copy link
Contributor

zachriggle commented Oct 30, 2018

Overall this looks good, and most of the heavy lifting is done! Cool stuff!

I've added some comments about convention (self.arch vs context.arch, general code style (e.g. number of spaces), tests (needs more and better tests), and documentation (no functions or methods have docs).

If you can clean this up a bit, we can get this merged!

var['unknown2']=48
return var

def packit(s,l=8):

This comment has been minimized.

Copy link
@zachriggle

zachriggle Oct 30, 2018

Contributor

This needs documentation and tests

var[i]=null
return var

def update_var(l):

This comment has been minimized.

Copy link
@zachriggle

zachriggle Oct 30, 2018

Contributor

This needs documentation and tests

'_wide_data'
]

def get_defaults(null):

This comment has been minimized.

Copy link
@zachriggle

zachriggle Oct 30, 2018

Contributor

Indentation is wrong here. I'd recommend looking at collections.defaultdict to avoid the need for this.

>>> fileStr.flags = 0xfbad1807
>>> fileStr._IO_buf_base = 0xcafebabe
>>> fileStr._IO_buf_end = 0xfacef00d
>>> payload = str(fileStr)

This comment has been minimized.

Copy link
@zachriggle

zachriggle Oct 30, 2018

Contributor

This should show what the payload looks like, so that there are actual tests.

>>> context.clear(arch='amd64')
>>> fileStr = FileStructure(0xdeadbeef)
>>> payload = fileStr.read(addr=0xcafebabe, size=100)

This comment has been minimized.

Copy link
@zachriggle

zachriggle Oct 30, 2018

Contributor

Show the payload

self['fileno'] = 1
return self.struntil('fileno')

def read(self,addr=0,size=0):

This comment has been minimized.

Copy link
@zachriggle

zachriggle Oct 30, 2018

Contributor

Needs docs and doctests

break
return structure[:len(structure)-1]

def write(self,addr=0,size=0):

This comment has been minimized.

Copy link
@zachriggle

zachriggle Oct 30, 2018

Contributor

Needs docs and doctests

def set_vars(self,item,value):
self[item]=value

def struntil(self,v):

This comment has been minimized.

Copy link
@zachriggle

zachriggle Oct 30, 2018

Contributor

Needs docs and doctests

structure=''
for val in self.vars_:
if isinstance(self[val],str):
if self.arch=='i386':

This comment has been minimized.

Copy link
@zachriggle

zachriggle Oct 30, 2018

Contributor

Do not use self.arch, use context.arch

return structure[:len(structure)-1]

def write(self,addr=0,size=0):
self['flags'] &=~8

This comment has been minimized.

Copy link
@zachriggle

zachriggle Oct 30, 2018

Contributor

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.

This comment has been minimized.

Copy link
@vigneshsrao

vigneshsrao Oct 31, 2018

Author Contributor

So should we remove the inheritance from dict altogether? Or should we just add documentation for the various fields?

This comment has been minimized.

Copy link
@zachriggle

zachriggle Oct 31, 2018

Contributor

Are there any places we leverage the dict inheritance other than to set self['field'] = value?

This comment has been minimized.

Copy link
@vigneshsrao

vigneshsrao Oct 31, 2018

Author Contributor

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.

@zachriggle

This comment has been minimized.

Copy link
Contributor

zachriggle commented Nov 1, 2018

@vigneshsrao

This comment has been minimized.

Copy link
Contributor Author

vigneshsrao commented Nov 1, 2018

Neat! Did not think of this before. I'll make the necessary changes as soon as possible. Thanks a lot!

@vigneshsrao vigneshsrao force-pushed the vigneshsrao:dev branch from 0215412 to 8dcd656 Nov 2, 2018
@vigneshsrao vigneshsrao force-pushed the vigneshsrao:dev branch from 8dcd656 to 3167ab5 Nov 2, 2018
@vigneshsrao

This comment has been minimized.

Copy link
Contributor Author

vigneshsrao commented Nov 2, 2018

Completed the necessary changes.

@orenht

This comment has been minimized.

Copy link

orenht commented Apr 25, 2019

Hi, are there any plans on getting this merged? :)

@vigneshsrao

This comment has been minimized.

Copy link
Contributor Author

vigneshsrao commented Jul 14, 2019

@zachriggle Are there any more changes to be made to the code?
If not can this be merged?

Arusekk added 3 commits Dec 8, 2019
also rename the module to lower case to keep consistency
@Arusekk Arusekk merged commit 59c7810 into Gallopsled:dev Dec 9, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.2%) to 60.338%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.