HTTP PDF Authors Gather Module: Use Msf::Exploit::Remote::HttpClient #8686

sempervictus · 2017-07-10T07:44:00Z

Replace Net::HTTP usage with proper Rex::Proto::Http::Client via
the Msf module mixin. Generate the request opts from the same URI
parsed URL string, execute a one shot GET request, disconencting
after reciept of results. Depending on the response code, either
pass back an empty StringIO or if its 200, a StringIO(res.body).

Verification

Before this commit:

(2017-07-10)03:42 (S:1 J:6)msf  auxiliary(http_pdf_authors) > exploit

[*] [2017.07.10-03:42:03] Processing 1 URLs...
[*] [2017.07.10-03:42:03] Downloading 'https://www.rapid7.com/docs/Product-Brief-Metasploit.pdf'
[*] [2017.07.10-03:42:03] HTTP 200 -- Downloaded PDF (99167 bytes)
[*] [2017.07.10-03:42:03] 100.00% done (1/1 files)

[*] [2017.07.10-03:42:03] Found no authors
[*] Auxiliary module execution completed

After:

(2017-07-10)03:42 (S:1 J:6)msf  auxiliary(http_pdf_authors) > reload
[*] Reloading module...
(2017-07-10)03:42 (S:1 J:6)msf  auxiliary(http_pdf_authors) > exploit

[*] [2017.07.10-03:42:18] Processing 1 URLs...
[*] [2017.07.10-03:42:18] Downloading 'https://www.rapid7.com/docs/Product-Brief-Metasploit.pdf'
[*] [2017.07.10-03:42:18] HTTP 200 -- Downloaded PDF (99167 bytes)
[*] [2017.07.10-03:42:18] 100.00% done (1/1 files)

[*] [2017.07.10-03:42:18] Found no authors
[*] Auxiliary module execution completed

Replace Net::HTTP usage with proper Rex::Proto::Http::Client via the Msf module mixin. Generate the request opts from the same URI parsed URL string, execute a one shot GET request, disconencting after reciept of results. Depending on the response code, either pass back an empty StringIO or if its 200, a StringIO(res.body).

To address the complexity which comes with the flexibility offered by Rex::Proto::Http::Client and its Msf mixin descendant, a simple process needs to be implemented for issuing a request using only the URL string in order to provide ease of access to users who may not have the time to study how these clients work in detail. Implement :request_opts_from_url in Msf's HttpClient mixin such as to extract the options required for :send_request_* from a URL string passed into the method. This approach reduces HTTP requests in the mixin to `send_request_raw(request_opts_from_url(url))` when `url` is just a string. Implement this approach in the http_pdf_authors gather module to further reduce infrastructure complexity around the simple need to acquire PDF files via HTTP/S. Testing: Local to this module only, and in Pry of course. Seems to work...

sempervictus · 2017-07-10T08:30:31Z

ping @bcook-r7 @egypt: the last commit before this comment addresses a need you guys mentioned for "simple access to complex HTTP subsystems" by just parsing out a URL string into the opts needed to send_request_*
Could you guys weigh in on the library side of the implementation? I'd like to get this landed so we can fixup all the other modules @bcoles mentioned in #8664

To round out implementation of a simple path for users to access HttpClient like Open or Net::HTTP, create :request_url method which takes a single URL parameter, uses :request_opts_from_url to build the request configuration for Rex::Proto::Http::Client, executes a GET request with it, and disconnects the client unless keepalive is specified as the second parameter to :request_url. Example usage of functionality is implemented in http_pdf_authors.

bcoles · 2017-07-11T20:35:10Z

I like the library changes.

Worth noting that each module that makes use of request_url should probably deregister_options 'RHOST', 'RPORT', 'VHOST'

Edit: Based on the appletv_display_image.rb module as an example, the following options should probably be de-registered in each module that makes use of request_url too.

      'HTTP::uri_encode_mode', 'HTTP::uri_full_url', 'HTTP::pad_method_uri_count',
      'HTTP::pad_uri_version_count', 'HTTP::pad_method_uri_type', 'HTTP::pad_uri_version_type',
      'HTTP::method_random_valid', 'HTTP::method_random_invalid', 'HTTP::method_random_case',
      'HTTP::uri_dir_self_reference', 'HTTP::uri_dir_fake_relative', 'HTTP::uri_use_backslashes',
      'HTTP::pad_fake_headers', 'HTTP::pad_fake_headers_count', 'HTTP::pad_get_params',
      'HTTP::pad_get_params_count', 'HTTP::pad_post_params', 'HTTP::pad_post_params_count',
      'HTTP::uri_fake_end', 'HTTP::uri_fake_params_start', 'HTTP::header_folding',
      'NTLM::UseNTLM2_session', 'NTLM::UseNTLMv2', 'NTLM::SendLM', 'NTLM::SendNTLM',
      'NTLM::SendSPN', 'NTLM::UseLMKey', 'DOMAIN', 'DigestAuthIIS', 'VHOST'

bcoles · 2017-07-11T21:01:47Z

lib/msf/core/exploit/http/client.rb

+  #
+  # Returns response from a simple URL call
+  def request_url(url, keepalive = false)
+    res = send_request_raw(request_options_from_url(url))


request_opts_from_url

Yeah, OK. This is why I may not be ideally suited to working at the interface level. Wilco

bcoles · 2017-07-11T21:06:59Z

lib/msf/core/exploit/http/client.rb

+  # Returns a hash of request opts from a URL string
+  def request_opts_from_url(url)
+    tgt = URI.parse(url)
+    opts = { 'rhost' => tgt.host, 'rport' => tgt.port, 'uri' => tgt.request_uri }


Presumably this method will always expect an absolute URL.

A URL of / is valid input for URI.parse, but tgt will have no scheme or host attributes.

Perhaps error handling should be performed here?

begin tgt = URI.parse(url) raise 'Invalid URL' unless tgt.scheme =~ %r{https?} raise 'Invalid URL' if tgt.host.to_s.eql? '' rescue => e print_error "Could not parse URL: #{e}" return end

... or something

You're entirely right, I should have pulled the full validation block you had going. Will adjust as requested, thx.

sempervictus · 2017-07-11T21:27:53Z

@bcoles: since your module is being used as a guinea pig for this, do you want me to continue abusing it in this PR, or turn this into another nightmare for Egypt and Sinn3r to dig thru in lib with no explicit exposure in a module? Didn't mean to hijack your efforts on improving this.

bcoles · 2017-07-11T21:32:16Z

@sempervictus I'm happy for you to keep using this module as a guinea pig.

I've re-written my module to make use of your library changes and patch a few other things. The code is much cleaner.

My only concern is that there's some issues in the underlying pdf-reader library, which I've since patched in my local repo, but will cause merge conflicts with this PR. One of the underlying issues is a hang with 100% CPU usage which is rather poor user experience.

sempervictus · 2017-07-11T21:43:57Z

Let's add your repo to the gemfile until the author adopts or replaces your fixes. Again, big ups on keeping the user base safe. Client side exploits against MSF are to be avoided at most costs.

bcoles · 2017-07-11T21:47:01Z

Ah, to clarify, I mean I've patched by rescuing. I haven't actually patched the lib.

Using Timeout ensures the maximum hang time is 10 seconds. This seemed like the safest approach, although perhaps a shorter time would be better.

SystemStackError and SyntaxError also need to be rescued specifically, as they aren't rescued by a typical rescue clause.

SyntaxError has to be rescued due to what seems like a poor design decision by the authors of the RC4 dependency.

  def read(data)
    Timeout.timeout(10) do
      reader = PDF::Reader.new data
      return parse reader
    end
  rescue PDF::Reader::MalformedPDFError
    print_error "Could not parse PDF: PDF is malformed (MalformedPDFError)"
    return
  rescue PDF::Reader::UnsupportedFeatureError
    print_error "Could not parse PDF: PDF contains unsupported features (UnsupportedFeatureError)"
    return
  rescue SystemStackError
    print_error "Could not parse PDF: PDF is malformed (SystemStackError)"
    return
  rescue SyntaxError
    print_error "Could not parse PDF: PDF is malformed (SyntaxError)"
    return
  rescue Timeout::Error
    print_error "Could not parse PDF: PDF is malformed (Timeout)"
    return
  rescue => e
    print_error "Could not parse PDF: Unhandled exception: #{e}"
    return
  end

… client mixin Updated PDF author metadata downloader to support the new methods.

busterb · 2017-08-14T05:47:28Z

Thanks @bcoles and @sempervictus. I added an additional commit and some error handling here based on @bcoles' updated work on this PR:

5d05ca1 - added http client 'download' method and updates to pdf author module from @bcoles

busterb · 2017-08-14T05:49:49Z

Release Notes

This improves error handling in the auxiliary/gather/http_pdf_authors module, and converts it to work over pivots, by moving much of its HTTP download logic into the HTTP client mixin. Now there are simplified HTTP client methods that other modules can use for this common functionality as well.

sempervictus changed the title ~~Use Msf::Exploit::Remote::HttpClient~~ HTTP PDF Authors Gather Module: Use Msf::Exploit::Remote::HttpClient Jul 10, 2017

sempervictus mentioned this pull request Jul 10, 2017

Add PDF link scraping to http_pdf_authors module #8664

Closed

bcoles mentioned this pull request Jul 11, 2017

Modules should not use Net::HTTP or Rex::Proto::Http::Client.new #8705

Open

bcoles reviewed Jul 11, 2017

View reviewed changes

fix request_ops per bcoles

7e487ec

busterb self-assigned this Aug 14, 2017

busterb merged commit 7e487ec into rapid7:master Aug 14, 2017

busterb pushed a commit that referenced this pull request Aug 14, 2017

Land #8686, add 'download' and simplified URI request methods to http…

2619321

… client mixin Updated PDF author metadata downloader to support the new methods.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HTTP PDF Authors Gather Module: Use Msf::Exploit::Remote::HttpClient #8686

HTTP PDF Authors Gather Module: Use Msf::Exploit::Remote::HttpClient #8686

sempervictus commented Jul 10, 2017 •

edited by busterb

sempervictus commented Jul 10, 2017

bcoles commented Jul 11, 2017 •

edited

bcoles Jul 11, 2017

sempervictus Jul 11, 2017

bcoles Jul 11, 2017

sempervictus Jul 11, 2017

sempervictus commented Jul 11, 2017

bcoles commented Jul 11, 2017

sempervictus commented Jul 11, 2017

bcoles commented Jul 11, 2017 •

edited

busterb commented Aug 14, 2017

busterb commented Aug 14, 2017

HTTP PDF Authors Gather Module: Use Msf::Exploit::Remote::HttpClient #8686

HTTP PDF Authors Gather Module: Use Msf::Exploit::Remote::HttpClient #8686

Conversation

sempervictus commented Jul 10, 2017 • edited by busterb

Verification

sempervictus commented Jul 10, 2017

bcoles commented Jul 11, 2017 • edited

bcoles Jul 11, 2017

Choose a reason for hiding this comment

sempervictus Jul 11, 2017

Choose a reason for hiding this comment

bcoles Jul 11, 2017

Choose a reason for hiding this comment

sempervictus Jul 11, 2017

Choose a reason for hiding this comment

sempervictus commented Jul 11, 2017

bcoles commented Jul 11, 2017

sempervictus commented Jul 11, 2017

bcoles commented Jul 11, 2017 • edited

busterb commented Aug 14, 2017

busterb commented Aug 14, 2017

Release Notes

sempervictus commented Jul 10, 2017 •

edited by busterb

bcoles commented Jul 11, 2017 •

edited

bcoles commented Jul 11, 2017 •

edited