Simplify. Document. Handle edge cases

Simplify detection logic.

Document testing method better

Ensure that body doesn't include canary cookie name too

Use full_uri in prints when possible

Author: jhart-r7

modules/auxiliary/scanner/http/allegro_rompager_misfortune_cookie.rb

@@ -40,12 +40,6 @@ def initialize(info = {})
         OptString.new('TARGETURI', [true, 'URI to test', '/'])
       ], Exploit::Remote::HttpClient
     )
-
-    register_advanced_options(
-      [
-        OptBool.new('REQUIRE_AUTH', [true, 'Require that the tested URI require authentication', false])
-      ], self.class
-    )
   end
 
   def check_host(_ip)
@@ -67,97 +61,106 @@ def run_host(ip)
     end
   end
 
-  def find_canary_uri
+  def check_response_fingerprint(res, fallback_status)
+    fp = http_fingerprint(response: res)
+    if /RomPager\/(?<version>[\d\.]+)/ =~ fp
+      vprint_status("#{peer} is RomPager #{version}")
+      if Gem::Version.new(version) < Gem::Version.new('4.34')
+        return Exploit::CheckCode::Detected
+      end
+    end
+    fallback_status
+  end
+
+  def find_canary
     vprint_status("#{peer} locating suitable canary URI")
     0.upto(4) do
       canary = '/' + Rex::Text.rand_text_alpha(16)
-      res = send_request_raw('uri' => normalize_uri(canary), 'method' => 'GET', 'headers' => headers)
-      # in most cases, the canary URI will not exist and will return a 404, but if everything under
-      # TARGETURI is protected by auth, that may be fine too
+      res = send_request_raw(
+        'uri' => normalize_uri(canary),
+        'method' => 'GET',
+        'headers' => headers
+      )
+      # in most cases, the canary URI will not exist and will return a 404, but
+      # if everything under TARGETURI is protected by auth, that may be fine
+      # too
       return canary if res.code == 401 || res.code == 404
     end
     nil
   end
 
   def headers
     {
-      'Referer' => datastore['SSL'] ? 'https' : 'http' + "://#{rhost}:#{rport}"
+      'Referer' => full_uri
     }
   end
 
-  def requires_auth?
-    res = send_request_raw(
-      'uri' => normalize_uri(target_uri.path.to_s),
-      'method' => 'GET',
-      'headers' => headers
-    )
-    return false unless res
-
-    http_fingerprint(response: res)
-    if res.code == 401
-      vprint_status("#{peer} requires authentication for #{target_uri.path}")
-      true
-    else
-      vprint_status("#{peer} does not require authentication for #{target_uri.path} -- code #{res.code}")
-      false
-    end
-  end
-
+  # To test for this vulnerability, we must first find a URI known to return
+  # a 404 (not found) which we will use as a canary.  This URI (for example,
+  # /foo) is then taken and used as the value for a carefully crafted cookie
+  # when making a request to the configured host+port+uri.  If the response
+  # is a 404 and the body includes the canary, it is likely that the cookie
+  # overwrote RomPager's concept of the requested URI, indicating that it is
+  # vulnerable.
   def test_misfortune
-    if datastore['REQUIRE_AUTH']
-      return Exploit::CheckCode::Unknown unless requires_auth?
-    end
-
-    # find a usable canary URI (one that 401/404s already)
-    unless canary = find_canary_uri
+    # find a usable canary URI (one that returns a 404 already)
+    unless (canary_value = find_canary)
       vprint_error("#{peer} Unable to find a suitable canary URI")
       return Exploit::CheckCode::Unknown
     end
 
-    # Make a request containing a malicious cookie with the canary value.
-    # If that canary shows up in the *body*, they are vulnerable
+    canary_cookie_name = 'C107373883'
+    canary_cookie = canary_cookie_name + "=#{canary_value};"
+
+    # Make a request containing a specific canary cookie name with the value set
+    # from the suitable canary value found above.
     res = send_request_raw(
       'uri' => normalize_uri(target_uri.path.to_s),
       'method' => 'GET',
-      'headers' => headers.merge('Cookie' => "C107373883=#{canary}")
+      'headers' => headers.merge('Cookie' => canary_cookie)
     )
 
     unless res
-      vprint_error("#{peer} no response")
+      vprint_error("#{full_uri} no response")
       return Exploit::CheckCode::Unknown
     end
 
-    # fingerprint because this is useful and also necessary if the canary is not
-    # in the body
-    fp = http_fingerprint(response: res)
-
-    unless res.body
-      vprint_status("#{peer} HTTP code #{res.code} had no body")
-      return Exploit::CheckCode::Unknown
+    unless res.code == 404
+      vprint_status("#{full_uri} unexpected HTTP code #{res.code} response")
+      return check_response_fingerprint(res, Exploit::CheckCode::Unknown)
     end
 
-    if res.body.include?(canary)
-      vprint_good("#{peer} HTTP code #{res.code} response contained canary URI #{canary}")
-      report_vuln(
-        host: rhost,
-        port: rport,
-        name: name,
-        refs: references
-      )
-      return Exploit::CheckCode::Appears
+    unless res.body
+      vprint_status("#{full_uri} HTTP code #{res.code} had no body")
+      return check_response_fingerprint(res, Exploit::CheckCode::Unknown)
     end
 
-    vprint_status("#{peer} HTTP code #{res.code} response did not contain canary URI #{canary}")
-    if /RomPager\/(?<version>[\d\.]+)/ =~ fp
-      vprint_status("#{peer} is RomPager #{version}")
-      if Gem::Version.new(version) < Gem::Version.new('4.34')
-        return Exploit::CheckCode::Detected
+    # If that canary *value* shows up in the *body*, then there are two possibilities:
+    #
+    # 1) If the canary cookie *name* is also in the *body*, it is likely that
+    # the endpoint is puppeting back our request to some extent and therefore
+    # it is expected that the canary cookie *value* would also be there.
+    # return Exploit::CheckCode::Unknown
+    #
+    # 2) If the canary cookie *name* is *not* in the *body*, return
+    # Exploit::CheckCode::Appears
+    if res.body.include?(canary_value)
+      if res.body.include?(canary_cookie_name)
+        vprint_status("#{full_uri} HTTP code #{res.code} response contained test cookie name #{canary_cookie_name}")
+        return check_response_fingerprint(res, Exploit::CheckCode::Unknown)
+      else
+        vprint_good("#{full_uri} HTTP code #{res.code} response contained canary cookie value #{canary_value} as URI")
+        report_vuln(
+          host: rhost,
+          port: rport,
+          name: name,
+          refs: references
+        )
+        return Exploit::CheckCode::Appears
       end
     end
 
-    # TODO: ensure that the canary page doesn't exist in the first place
-    # (returns a 404), and then ensure that the malcious request with the
-    # carary in the cookie then returns a 404.
-    Exploit::CheckCode::Safe
+    vprint_status("#{full_uri} HTTP code #{res.code} response did not contain canary cookie value #{canary_value} as URI")
+    check_response_fingerprint(res, Exploit::CheckCode::Safe)
   end
 end