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

Feature: Search nearest gadgets #76

Merged
merged 14 commits into from
May 4, 2019

Conversation

umutoztunc
Copy link
Contributor

This adds a new option -n/--near FUNCTIONS/FILE to implement and close #16. However, it does not work with -b, --build-id BuildID and only works if the libc file is given since we need to retrieve function offsets. However, it can be implemented later by adding function tables in ruby scripts under lib\builds folder.

It also works with both -l/--level OUTPUT_LEVEL and -r/--raw options properly.

For now, it just reorders the result and prints the closer gadgets first. However, we can introduce a threshold option in the future to make it print less functions. For example, print only if the distance is less than 0x1000.

This option expects a comma seperated list of function names or a file.

Examples:

  • one_gadget libc.so.6 --near puts,printf,scanf --level 3
  • one_gadget libc.so.6 --near "putchar, fscanf" --raw --level 2
  • one_gadget libc.so.6 --near ./vuln
  • one_gadget libc.so.6 --near vuln --raw

Screenshots:
near_functions

near_file

near_raw

Returns the names of functions from the given file's global offset table
as an array of strings.
Returns a Hash object which maps function names to their offsets.
For every function given as a comma seperated list or for all the
functions from a file's global offset table, reorder the found gadgets
by their distance to each function and print the result.
@david942j
Copy link
Owner

david942j commented Apr 29, 2019

Awesome! Thanks for your contribution.
Simply reorder the gadgets is good to me, I have some comments in a quick review:

  1. Please make sure the tests pass by executing bundle exec rake. CI shows there are coding style issues.
  2. Show functions' offset (maybe like "Gadegts near exit(0x31337)") so users can know how near are the gadgets.
  3. Add tests (under spec/) for testing this new feature and ensure 100% code coverage.

Feel free to comment if you have any questions.

@umutoztunc
Copy link
Contributor Author

I'm glad that you liked it!
Sorry for the coding style issues. I am new to ruby development and I was confused about using bundle or rake commands. I just tested the changes using rake spec and it didn't mention the style issues. Now, I resolved those coding style issues locally. However, even the fresh clone of the repo does not result in 100% code coverage.

Here is the output of bundle exec rake right after cloning the repo:

Running RuboCop...
Inspecting 39 files
.......................................

39 files inspected, no offenses detected
/usr/bin/ruby2.5 -I/var/lib/gems/2.5.0/gems/rspec-core-3.8.0/lib:/usr/share/rubygems-integration/all/gems/rspec-support-3.8.0/lib /var/lib/gems/2.5.0/gems/rspec-core-3.8.0/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb

Randomized with seed 34893
.......................FFF................F........F.........

Failures:

  1) one_gadget_aarch64 from file libc-2.24
     Failure/Error: expect(OneGadget.gadgets(file: path, force_file: true)).to eq [0x3c934, 0x3c970]
     
       expected: [248116, 248176]
            got: []
     
       (compared using ==)
     # ./spec/one_gadget_aarch64_spec.rb:17:in `block (3 levels) in <top (required)>'

  2) one_gadget_aarch64 from file libc-2.23
     Failure/Error:
       expect(OneGadget.gadgets(file: path, force_file: true, level: 1))
         .to eq [0x3d6d0, 0x3d6dc, 0x3d718, 0x60c1c, 0x60c20]
     
       expected: [251600, 251612, 251672, 396316, 396320]
            got: []
     
       (compared using ==)
     # ./spec/one_gadget_aarch64_spec.rb:11:in `block (3 levels) in <top (required)>'

  3) one_gadget_aarch64 from file libc-2.27
     Failure/Error: expect(OneGadget.gadgets(file: path, force_file: true)).to eq [0x3f160, 0x3f184, 0x3f1a8, 0x63e90]
     
       expected: [258400, 258436, 258472, 409232]
            got: []
     
       (compared using ==)
     # ./spec/one_gadget_aarch64_spec.rb:22:in `block (3 levels) in <top (required)>'

  4) OneGadget::Update cache_file
     Failure/Error: expect(described_class.__send__(:cache_file)).to be nil
     
       expected nil
            got #<String:47327747199500> => "/tmp/one_gadget/update20190429-2670-rueg8y"
     # ./spec/update_spec.rb:29:in `block (3 levels) in <top (required)>'
     # ./spec/update_spec.rb:15:in `block (4 levels) in <top (required)>'
     # ./spec/update_spec.rb:13:in `block (3 levels) in <top (required)>'
     # ./spec/update_spec.rb:26:in `block (2 levels) in <top (required)>'

  5) one_gadget_amd64 from file not glibc
     Failure/Error:
             expect { hook_logger { OneGadget.gadgets(file: '/bin/ls') } }.to output(<<-EOS.strip).to_stdout
       [OneGadget] ArgumentError: File "/bin/ls" doesn't contain string "/bin/sh", not glibc?
             EOS
     
       expected block to output "[OneGadget] ArgumentError: File \"/bin/ls\" doesn't contain string \"/bin/sh\", not glibc?" to stdout, but output "[OneGadget] ArgumentError: File \"/usr/bin/ls\" doesn't contain string \"/bin/sh\", not glibc?"
     # ./spec/one_gadget_amd64_spec.rb:41:in `block (3 levels) in <top (required)>'

Finished in 7.13 seconds (files took 0.28136 seconds to load)
61 examples, 5 failures

Failed examples:

rspec ./spec/one_gadget_aarch64_spec.rb:15 # one_gadget_aarch64 from file libc-2.24
rspec ./spec/one_gadget_aarch64_spec.rb:9 # one_gadget_aarch64 from file libc-2.23
rspec ./spec/one_gadget_aarch64_spec.rb:20 # one_gadget_aarch64 from file libc-2.27
rspec ./spec/update_spec.rb:24 # OneGadget::Update cache_file
rspec ./spec/one_gadget_amd64_spec.rb:40 # one_gadget_amd64 from file not glibc

Randomized with seed 34893

Coverage report generated for RSpec to /root/one_gadget/coverage. 847 / 861 LOC (98.37%) covered.
/usr/bin/ruby2.5 -I/var/lib/gems/2.5.0/gems/rspec-core-3.8.0/lib:/usr/share/rubygems-integration/all/gems/rspec-support-3.8.0/lib /var/lib/gems/2.5.0/gems/rspec-core-3.8.0/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb failed

Why do I get these failures locally?

@david942j
Copy link
Owner

david942j commented Apr 29, 2019

bundle is for creating a virtual environment that provides specific version of gems described in Gemfile.lock.
If your system-wise gems have exactly same version as versions in Gemfile.lock, then bundle exec rake should have almost same behavior as rake.

Simply execute rake then both linter and tests will be executed (see Rakefile for registered rake commands).

For the first three failures, I guess it's because you don't have objdump-multiarch installed.
For the fourth failure, no idea why, I will try to figure it out later.
For the latest one, just because your ls is under /usr/bin but not expected /bin, you can simply ignore this failure and I will fix it.

Update: the fourth failure is because you are root, you can ignore it as well and I will think how to make the test more robust

@umutoztunc
Copy link
Contributor Author

Thank you for clarifying bundle.

I have made the changes you requested in my local repository. The only thing left is adding tests under spec folder. I have analyzed the existing tests and noticed that you call OneGadget.gadgets function with different parameters and compare the result to a predefined list of expected values. However, I did not implement --near option as a parameter to OneGadget.gadgets. Thus, I am a little bit confused. How should I implement the tests for this feature? I can put the reordering process inside OneGadget.gadgets and pass an offset as a parameter. So that, we can just iterate over the function_offsets object and call OneGadget.gadgets function with the corresponding function's offset, then print the result. Another option might be putting everything inside OneGadget.gadgets function and changing its return type so that we can print all the results, but I am not sure if it is a good idea.

How should we move forward from this point?

@david942j
Copy link
Owner

You can have the test in bin_spec.rb
I have thought about moving code in bin/one_gadget to lib/, but haven't had time for refactoring it.
Anyway, put the new test in the bin_spec and invoke --near option as the exists --help test does.

@umutoztunc
Copy link
Contributor Author

umutoztunc commented Apr 30, 2019

By the way, you were right about objdump multiarch stuff. It was missing in my environment. I thought it would give me a warning if it was missing. Anyway, it is fixed since I installed binutils-multiarch.

Now, I am adding tests but in order to test the --near option with a file, I am using /bin/ls. However, the output is too long. Maybe, we should create a test file with a few functions in its got so that the testing code would take less space. What do you think about this?

UPDATE: I guess using /bin/ls in test is not a good idea since the file can differ across operating systems etc.

This commit adds two tests for testing --near feature option. One for
testing with a list of functions and the other for testing with a file.
Also, it adds a new file called testNearFile.elf for testing purposes.
@david942j
Copy link
Owner

Please discard the changes of README.md except the help message.
Add a skip windows statement in the test you added to prevent failing on appveyor.

@umutoztunc
Copy link
Contributor Author

I guess everything is okay now. I hope my future contributions will be more efficient.

@david942j
Copy link
Owner

Looks good now, I will find a time to test it manually and merge it if I don't find any bugs.

@david942j
Copy link
Owner

david942j commented May 4, 2019

  • Added some tests (and changed names) for helper functions
  • Changed the color and form of 'Gadgets near..'
    This PR will be merged once the CI finished and passed.

螢幕快照 2019-05-04 14 12 54

@david942j david942j merged commit 853dc84 into david942j:master May 4, 2019
@umutoztunc
Copy link
Contributor Author

Looks great! Maybe, we should add some example usage of this feature in README.md since there is nothing other than the feature's description. People might get confused a bit. Therefore, a few short examples might be helpful to clarify it.

@david942j
Copy link
Owner

Yes I'm working on it, no worries :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: search nearest one gadgets
2 participants