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

A user privilege elevation vulnerability in the latest version of gitblit #1410

Closed
YYHYlh opened this issue Feb 28, 2022 · 7 comments · Fixed by #1411
Closed

A user privilege elevation vulnerability in the latest version of gitblit #1410

YYHYlh opened this issue Feb 28, 2022 · 7 comments · Fixed by #1411

Comments

@YYHYlh
Copy link

YYHYlh commented Feb 28, 2022

Hello, I tried to contact the developers of your product but did not get a response, so I decided to raise the vulnerability to you in the issue, hoping that you can fix it as soon as possible to avoid a wider impact of the vulnerability.

Principle of the vulnerability

Gitblit uses file storage to manage user information, passwords, account types, and permissions. When a user with low privileges modifies their information, if they use line breaks and space characters, they can create new users or assign higher
higher privileges.
The relevant code logic is in the write function of com.gitblit.ConfigUserService. The reason for the problem is that gitblit does not do a checksum on the characters entered by the user, and malicious characters are printed directly in the file, causing gitblit to parse the file incorrectly when reading it.
The location where users are saved is in data/users.conf.
The default users.conf is as follows.

[user "admin"]
    password = admin
    role = "#admin"
    role = "#notfederated"

The user name is admin, the password is admin, and the user's permissions are admin permissions, and the file will change as the user logs in. After logging in once the file reads

[user "admin"]
    accountType = LOCAL
    cookie = aad70b95ca5ffe59c88cd567b91999b263acb659
    emailMeOnMyTicketChanges = true
    password = PBKDF2:$0$33c135e0e31a085587920e0981401bc34169cc1460a321d8f748969939383ce76c403eda5015281d2ff3b2203c5c2397154662b3219ba979476d9e7bc47c29f9
    role = "#admin"
    role = "#notfederated"

If there is a new user, a new user will be created below the user, and which will be accompanied by the user's emailAddress information, if the attacker in the modification of their own emailAddress, the emailAddress set to xxx.@qq.com\n\trole = "#admin"\n[user "other"],you can modify the permissions of their own user to admin.

Vulnerability recurrence

  1. The attacker has an account with no privileges, username test, password test1, and privileges None, and the current users.conf is.
[user "admin"]
    accountType = LOCAL
    cookie = aad70b95ca5ffe59c88cd567b91999b263acb659
    emailMeOnMyTicketChanges = true
    password = PBKDF2:$0$33c135e0e31a085587920e0981401bc34169cc1460a321d8f748969939383ce76c403eda5015281d2ff3b2203c5c2397154662b3219ba979476d9e7bc47c29f9
    role = "#admin"
    role = "#notfederated"
[user "test"]
    accountType = LOCAL
    cookie = 513b0430e84cbccad003a3b8e5c614797a311e70
    emailAddress = 111@qq.com
    emailMeOnMyTicketChanges = true
    password = PBKDF2:$0$ec5a6828b39c0ec958c7f70861b0bbc7aa3b74e45ebfe8a0ffc0689923b517e96a8d8039392631369b6cd2d6742771d582f0e4cb24c254cdbdc126c7ebcd88f4
    role = "#none"
  1. After logging in, click on Profile->Preferences in order
    image
    Turn on burpsuite's blocking feature, click Save, and block the request with burpsuite.

  2. In burpsuite, make changes to the request.
    image

Modify the value of emailAddress%3Atext to 111@qq.com\n\trole = "#admin"\n[user "other"] after url encoding. The[user "other"]at the end of the payload is to avoid the impact of the original role = "#none".
The encoding can be done using burpsuite's Decode function

image

  1. After modifying the request body and sending the request, users.conf changes to the following state.
    image

You can see that the test user has become admin privileges. Refreshing the page, at this point the test user has full access to gitblit, and can see all Git repositories and manage all users and teams

image

@flaix
Copy link
Member

flaix commented Mar 1, 2022

Excellent. Thank you.
Just for me to know, what ways did you try to contact the developers that were unsuccessful?

@YYHYlh
Copy link
Author

YYHYlh commented Mar 2, 2022

Excellent. Thank you. Just for me to know, what ways did you try to contact the developers that were unsuccessful?

I found some contacts on your official website and in the Github Readme at

  • google group (not used by anyone for a long time)
  • Official Twitter (not used by anyone for a long time)
  • A private tweet @jamesmoger (recently updated), but his tweet doesn't allow private messages, so I left a message for his latest tweet, but haven't received a reply yet.

I think you guys could update your latest contact information, preferably using a private email for receiving some of the more private security questions.

@flaix flaix linked a pull request Mar 13, 2022 that will close this issue
@flaix
Copy link
Member

flaix commented Mar 13, 2022

@YYHYlh , would you like to review and test the pull request before a release?

flaix added a commit to flaix/gitblit that referenced this issue Mar 13, 2022
The `StoredUserConfig` only escaped the escape character, i.e. backslash.
But it does not escape control characters like tab or newline. This
introduces a vulnerability where an attacker can create new entries
in their user account and create new accounts.
In addition, other characters are also not properly handled. Field values
with a comment character need to be quoted. This only happens for the
`#` character and only when the value starts with it. Also the quote
is note escaped in values.

This change completely rewrites the `escape` method of `StoredUserConfig`.
It takes care of properly escaping characters that need escaping for the
git configuration file format.

This fixes gitblit-org#1410
@YYHYlh
Copy link
Author

YYHYlh commented Mar 14, 2022

@YYHYlh , would you like to review and test the pull request before a release?

I don't think it's needed anymore, because I'm not quite sure how the project compiles. But I can't inject into the config file anymore without using the characters you disabled, and I think that part of the code is already safe. I will test again when your new version is released

@flaix flaix closed this as completed in 9b4afad Mar 14, 2022
@flaix
Copy link
Member

flaix commented Mar 14, 2022

I have merged the fix into the master branch, which means it will be built with the next nightly build.
Should you happen to use Docker images, you can use the latest nightly Docker image to test the fix. Otherwise you could use the artefact attached to the last nightly build.

@YYHYlh
Copy link
Author

YYHYlh commented Mar 24, 2022

I have merged the fix into the master branch, which means it will be built with the next nightly build. Should you happen to use Docker images, you can use the latest nightly Docker image to test the fix. Otherwise you could use the artefact attached to the last nightly build.

I think the patch has fixed the vulnerability successfully. I will contact you promptly if I find any new vulnerabilities after that.

@DonnyDong2008
Copy link

I met an issue today when I was trying to login Gitblit 1.9.2 using a local admin account. It failed login and prompt me invalid user name or password. Then I used another windows domain account logged in which was granted admin privilege. Then I found that the user list is empty. I logged into the server where gitblit is installed, and found that the users.conf is empty. I don't know why. Just found some exception in log about failed to read users.conf. I just upgraded to 1.9.3 right now. Hope this issue will never happen again. Please refer to below log:
2022-07-20 14:35:58 [ERROR] Failed to read D:\Program Files\gitblit-1.9.2\data\users.conf
org.eclipse.jgit.errors.ConfigInvalidException: Cannot read file D:\Program Files\gitblit-1.9.2\data\users.conf
at org.eclipse.jgit.storage.file.FileBasedConfig.load(FileBasedConfig.java:194)
at com.gitblit.ConfigUserService.read(ConfigUserService.java:885)
at com.gitblit.ConfigUserService.getUserModel(ConfigUserService.java:190)

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

Successfully merging a pull request may close this issue.

3 participants