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

SAP ConfigServlet remote code execution exploit #1759

Conversation

@andrewkabai
Copy link
Contributor

@andrewkabai andrewkabai commented Apr 24, 2013

This module allows remote code execution on the target system
by using operating system commands (CmdStagerVBS) through the
SAP ConfigServlet OS command execution vulnerability,
and of course without any authentication.

The vulnerability was discovered by ERPScan's team, it was presented on Hacker Halted 2012 conference:
http://erpscan.com/wp-content/uploads/2012/11/Breaking-SAP-Portal-HackerHalted-2012.pdf

This module is an exploit version of the previously merged auxiliary module:
#1751

andrewkabai added 13 commits Apr 20, 2013
the original aux module will be the base of the exploit module
switch to Msf::Exploit, change the necessary declarations, start to
change the exploitation process
commas in the HTTP queries are not allowed but the VBS stager contains
some, therefore it was necessary to find a way to echo out commas
without directly use them.
thanks to Laszlo Toth to help me figure out this windows command line
trick.
@jvazquez-r7
Copy link
Contributor

@jvazquez-r7 jvazquez-r7 commented Apr 24, 2013

Thanks @andrewkabai , as said via email will do code review of this one asap, hopefully along the day of today!

@L1ghtn1ng
Copy link
Contributor

@L1ghtn1ng L1ghtn1ng commented Apr 24, 2013

Hi @andrewkabai you seem to be missing the license information at the top of the module. You can find that in another module at the very top. Also if not done so already please run your module past msftidy thank you

andrewkabai added 4 commits Apr 24, 2013
add license
remove one unnecessary tab to make msftidy happy
ARCH_CMD was implemented in previous version of this code.
@jvazquez-r7
Copy link
Contributor

@jvazquez-r7 jvazquez-r7 commented Apr 24, 2013

Since it's an Exploit module a Rank must be provided. GreatRanking sounds like the good ranking here.

'Targets' =>
[
[
'Windows generic',

This comment has been minimized.

@jvazquez-r7

jvazquez-r7 Apr 24, 2013
Contributor

Please provide the SAP Portal version you've tested the module against if possible, thanks!

This comment has been minimized.

@andrewkabai

andrewkabai Apr 26, 2013
Author Contributor

it is now included in the description

commands.each do |command|
timeout = 20
if command.include?(".vbs") and command.include?(",")
# becasue the comma is bad character and the VBS stager contains commas it is necessary to "create" commas withouth directly using them

This comment has been minimized.

@jvazquez-r7

jvazquez-r7 Apr 24, 2013
Contributor

Typos => becasue and withouth

}, timeout)

if !res
print_error("#{rhost}:#{rport} - Exploit failed.")

This comment has been minimized.

@jvazquez-r7

jvazquez-r7 Apr 24, 2013
Contributor

Do

fail_with(Exploit::Failure::Unknown, "#{rhost}:#{rport} - Exploit failed.")

And delete the print_error

end

if res.code != 200
print_error("#{rhost}:#{rport} - Exploit failed.")

This comment has been minimized.

@jvazquez-r7

jvazquez-r7 Apr 24, 2013
Contributor

Delete

if res.code != 200
print_error("#{rhost}:#{rport} - Exploit failed.")
vprint_error("#{rhost}:#{rport} - Output: #{res.body}")
fail_with(Exploit::Failure::UnexpectedReply)

This comment has been minimized.

@jvazquez-r7

jvazquez-r7 Apr 24, 2013
Contributor

fail_with(Exploit::Failure::UnexpectedReply, "#{rhost}:#{rport} - Exploit failed.")
fail_with(Exploit::Failure::UnexpectedReply)
end
rescue ::Rex::ConnectionError
print_error("#{rhost}:#{rport} - Failed to connect to the server")

This comment has been minimized.

@jvazquez-r7

jvazquez-r7 Apr 24, 2013
Contributor

Delete print_error

end
rescue ::Rex::ConnectionError
print_error("#{rhost}:#{rport} - Failed to connect to the server")
fail_with(Exploit::Failure::Unreachable)

This comment has been minimized.

@jvazquez-r7

jvazquez-r7 Apr 24, 2013
Contributor

fail_with(Exploit::Failure::Unreachable, "#{rhost}:#{rport} - Failed to connect to the server")
end

if not res.body.include?("Process created")
print_error("#{rhost}:#{rport} - Exploit failed.")

This comment has been minimized.

@jvazquez-r7

jvazquez-r7 Apr 24, 2013
Contributor

Delete

if not res.body.include?("Process created")
print_error("#{rhost}:#{rport} - Exploit failed.")
vprint_error("#{rhost}:#{rport} - Output: #{res.body}")
fail_with(Exploit::Failure::PayloadFailed)

This comment has been minimized.

@jvazquez-r7

jvazquez-r7 Apr 24, 2013
Contributor

fail_with(Exploit::Failure::PayloadFailed, "#{rhost}:#{rport} - Exploit failed.")

This comment has been minimized.

@jvazquez-r7

jvazquez-r7 Apr 24, 2013
Contributor

fail_with(Exploit::Failure::PayloadFailed, "#{rhost}:#{rport} - Exploit failed.")
@jvazquez-r7
Copy link
Contributor

@jvazquez-r7 jvazquez-r7 commented Apr 25, 2013

Last things:

  • Would be nice to add a "def check" function if possible. Looks like should not be super difficult to determine, at least, if the affected servlet is responding.
  • Would be nice to use the FileDropper mixin to delete (or try to delete) dropped files to file system. Just grep "FileDropper" in the modules/exploits folder to look for usage samples.

Feel free to ask if you've doubts or anything to comment about the proposed feedback!

@andrewkabai
Copy link
Contributor Author

@andrewkabai andrewkabai commented Apr 25, 2013

most of the things are now fixed.
instead of FileDropper mixin I think the CmdStagerVBS built in cleanup functionality could be good as well.
although it is now disabled (:nodelete => true parameter in execute_cmdstager call) in my module but if you agree I would like make it configurable through a DELEFILES registered option.

@jvazquez-r7
Copy link
Contributor

@jvazquez-r7 jvazquez-r7 commented Apr 25, 2013

@andrewkabai thanks for the code update! Yeah the CMD Stager cleanup would work also fine, just thought you weren't enable it because of something. Yeah adding a datastore option to allow the user to enable or disable it would work fine. But please, enable it by default :) (verify it works as expected please :))

@jvazquez-r7
Copy link
Contributor

@jvazquez-r7 jvazquez-r7 commented Apr 25, 2013

Also have into account the CMDStager cleanup won't delete the dropped EXE, just the "vbs" and the "b64" files. Still would be useful to try to delete the "exe" through the FileDropper. I say try because prolly the delete will fail :) but in that case the user will be warned to delete it by himself, which is awesome.

@jvazquez-r7
Copy link
Contributor

@jvazquez-r7 jvazquez-r7 commented Apr 25, 2013

Also now there is OSVDB available: 92704

Would be nice to add it to references! :)

@kernelsmith
Copy link
Contributor

@kernelsmith kernelsmith commented Apr 25, 2013

@jvazquez-r7 what do u think of this #1764
It could be used in situations like this

@jvazquez-r7
Copy link
Contributor

@jvazquez-r7 jvazquez-r7 commented Apr 25, 2013

Looks like could be useful to put something like that in the FileDropper mixin. Anyway should be an option and the user warned about what files couldn't be deleted after a first try (just my opinion). Mainly, because prolly there is an action required from the user, like migrate to another process, in order to be able to delete some files.

@andrewkabai please don't be annoyed because of this off topic comments. they are more related to #1764

andrewkabai added 2 commits Apr 25, 2013
register DELETE_FILES advanced option to take control of the cleanup
functionality of CmdStagerVBS and FileDropper, implement the necessary
changes
# using the following command line trick it is possible to echo commas into the right places
command.gsub!(",", "%i")
command = "cmd /c FOR /F \"usebackq tokens=2 delims=)\" %i IN (\`\"ping -n 1 127.0.0.1| findstr )\"\`) DO " + command
if command.include?("shell.run")

This comment has been minimized.

@jvazquez-r7

jvazquez-r7 Apr 26, 2013
Contributor

This code is a little difficult to read I would do something like:

if datastore['DELETE_FILES'] and command =~ /shell\.run \"(.*)\"/
    register_file_for_cleanup($1)
end
if command.include?(".vbs") and command.include?(",")
    # because the comma is bad character and the VBS stager contains commas it is necessary to "create" commas without directly using them
    # using the following command line trick it is possible to echo commas into the right places
    command.gsub!(",", "%i")
    command = "cmd /c FOR /F \"usebackq tokens=2 delims=)\" %i IN (\`\"ping -n 1 127.0.0.1| findstr )\"\`) DO " + command
else
    command = "cmd /c " + command
end

Feel free to change if it also has sense for you :)

@jvazquez-r7
Copy link
Contributor

@jvazquez-r7 jvazquez-r7 commented Apr 26, 2013

And I think after fix the last comment and get the last data from @andrewkabai for verification it's ready to go :) good work @andrewkabai !

@andrewkabai
Copy link
Contributor Author

@andrewkabai andrewkabai commented Apr 26, 2013

hi @jvazquez-r7 , I mad the necessary changes and the final validation as well. I'll share the results with you via email.

@jvazquez-r7 jvazquez-r7 merged commit 5839e7b into rapid7:master Apr 26, 2013
1 check passed
1 check passed
default The Travis build passed
Details
@jvazquez-r7
Copy link
Contributor

@jvazquez-r7 jvazquez-r7 commented Apr 26, 2013

Verification data (logs and pcap) sent by @andrewkabai via email. Looks fine. Merged! Thanks @andrewkabai for an awesome contribution!

Last minor cleanup at 99b4620

@coveralls
Copy link

@coveralls coveralls commented Jul 4, 2014

Coverage Status

Changes Unknown when pulling 5839e7b on andrewkabai:module/exploit_sap_configservlet_exec_noauth into * on rapid7:master*.

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

Successfully merging this pull request may close these issues.

None yet

5 participants