5 ms·

Gitlab – Static passwords set during OmniAuth-based registration (CVE-2022-1162)

66 points by altharaz 4y ago

thcipriani 4y ago
To save folks some digging on what exactly this means—it's exactly what it sounds like: https://gitlab.com/gitlab-org/gitlab/-/commit/e2fb87ec5d4e235d6b83454980cec9c049849a1c#f4d654b98cc11d931e3f77ee61318adc95a52f12 https://gitlab.com/gitlab-org/gitlab/-/commit/e2fb87ec5d4e23...
- altharaz 4y ago
  The hardcoded password seems to be: Gitlab::Password.test_default(21) => "123qweQWE!@#000000000"
- eek2121 4y ago
  Do they not have a code review process?
  - mdaniel 4y ago
    https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318 https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318 and I actually suspected it was a matter of the change hiding in an absolute sea of diffs, but there's a comment on the file right below the change: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318/diffs#diff-content-f4d654b98cc11d931e3f77ee61318adc95a52f12 https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318/... > (10 Jan, 2022 1 commit) JH need more complex passwords > (30 Mar, 2022 1 commit) Revert "JH need more complex passwords" oops --- In case others are wondering what's up with the JiHu label and its matching "gitlab-jh" group: https://about.gitlab.com/handbook/ceo/chief-of-staff-team/jihu-support/ https://about.gitlab.com/handbook/ceo/chief-of-staff-team/ji...
    - bloomburger 4y ago
      Why has the person who wrote this vuln, Zhu Shung, not made any commits/contributions since they pushed this out two months ago?: https://gitlab.com/memorycancel https://gitlab.com/memorycancel Can somebody who what they were trying to do here opine on if this might have been malicious or was more likely just a honest mistake?
    - xPaw 4y ago
      What I find mildly curious, that's also the only place where a length was provided as an argument into `Gitlab::Password.test_default`.
    - altcognito 4y ago
      I thought I had an account there for a while, I don't know how long (2-3 years?), and all of a sudden I got a password reset out of the blue regarding this particular change. No other emails in my history regarding gitlab whatsoever, so it's even possible I didn't have an account at all. I go over to the website try to login with my gmail account via SSO which fails because I "already have an account". So I proceed to reset password via email alone. After I'm in it tells me I've had an account since January 22nd 2022. Super unlikely i created the account this year, so I don't know what's going on over there, but it's not accurate bookkeeping.
    - fy20 4y ago
      Is there a better way to catch errors like this? Looking through the PR it looks like this file was accidentally changed, I assume with a project wide search and replace. I could easily imagine myself missing this when reviewing the PR "oh it's just changing a whole bunch of specs, go ahead".
      - NavinF 4y ago
        An integration test that creates dummy accounts using every method including SSO and then attempts to bruteforce the password should find “12345678” within an hour. I think a test like this would also have found the dropbox and macos bugs that let you login to any account by using an empty password: https://techcrunch.com/2011/06/20/dropbox-security-bug-made-passwords-optional-for-four-hours/ https://techcrunch.com/2011/06/20/dropbox-security-bug-made-... https://arstechnica.com/information-technology/2017/11/macos-bug-lets-you-log-in-as-admin-with-no-password-required/ https://arstechnica.com/information-technology/2017/11/macos... Edit: Oh, the password was "123qweQWE!@#000000000". Technically doable with an efficient password cracker that favors common patterns. zxcvbn’s entropy estimate says it will take 10^10.5 guesses. That’s 1 week at 50k/s. That’s a hell of an integration test for most software. https://lowe.github.io/tryzxcvbn/ https://lowe.github.io/tryzxcvbn/
        dewey 4y ago
        > then attempts to bruteforce the password should find “12345678” within an hour But only if there's no rate limiting or increasing timeouts for wrong passwords which in most cases exists.
        NavinF 4y ago
        Right, but you can bypass that in a testing environment.
        dewey 4y ago
        Which then opens you up to exactly that class of bugs that also caused this issue. Having test specific code and feature flags and then testing a tweaked version isn't really covering all the cases then. Just like in this case where a hardcoded password was set to maybe log in through a test based on the naming "test_default".
    - theptip 4y ago
      Thanks for doing the issue sleuthing. This is an excruciatingly bad look. You'd have thought with all the code-owner functionality that GL has, they would lock down the `/lib/gitlab/auth/` files to require a security engineer to give additional signoff on top of a normal review. It looks like anyone at Gitlab can approve changes to the auth code (except LDAP): https://gitlab.com/gitlab-org/gitlab/-/blob/master/.gitlab/CODEOWNERS#L58 https://gitlab.com/gitlab-org/gitlab/-/blob/master/.gitlab/C... which is terrifying if true.
      - mdaniel 4y ago
        > to require a security engineer to give additional signoff on top of a normal review Like this? > cc @gitlab-com/gl-security/appsec https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#note_756311666 https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#... I do so desperately hope it doesn't come across as throwing shade, because hindsight-2020-etc, but I do also think there was some kind of weird process breakdown here because this change somehow slipped past a "4 eyes" and an appsec review phase
        deleted 4y ago
        [deleted]
      - nightpool 4y ago
        This was already required in this case, you can see in the comments that an additional AppSec review was done and the appsec team signed off on the MR above and beyond the normal code review process.
        bloomburger 4y ago
        And two of those appsec reviewers are now out of office for the next two weeks…
        orojackson 4y ago
        Heh, was this you [1]? Pretty much asking the same good questions you brought up in a different post. Additionally, I see that the Senior Director of Engineering, Tim Zallmann, has left a bunch of GitLab project repos about 14 hours ago as of this writing. He was one of the folks who tried pinging [3] Mr. Coutable (he's one of the reviewers that's currently OOO). The ping is likely regarding the discovery of the security vulnerability. [1] https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#note_898315619 https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#... [2] https://gitlab.com/users/timzallmann/activity https://gitlab.com/users/timzallmann/activity [3] https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#note_893221370 https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#...
krebsonsecurity 4y ago
This appears to be related. One Github user shared an alert they got today, two days after connecting their Github account to Gitlab. Something about an app added to the account. Their Github has 2fa turned on and a very strong password: https://twitter.com/briankrebs/status/1509910113716514822 https://twitter.com/briankrebs/status/1509910113716514822

66 points by altharaz 4y ago

To save folks some digging on what exactly this means—it's exactly what it sounds like: https://gitlab.com/gitlab-org/gitlab/-/commit/e2fb87ec5d4e235d6b83454980cec9c049849a1c#f4d654b98cc11d931e3f77ee61318adc95a52f12 https://gitlab.com/gitlab-org/gitlab/-/commit/e2fb87ec5d4e23...

altharaz 4y ago
The hardcoded password seems to be: Gitlab::Password.test_default(21) => "123qweQWE!@#000000000"
eek2121 4y ago
Do they not have a code review process?
- mdaniel 4y ago
  https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318 https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318 and I actually suspected it was a matter of the change hiding in an absolute sea of diffs, but there's a comment on the file right below the change: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318/diffs#diff-content-f4d654b98cc11d931e3f77ee61318adc95a52f12 https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318/... > (10 Jan, 2022 1 commit) JH need more complex passwords > (30 Mar, 2022 1 commit) Revert "JH need more complex passwords" oops --- In case others are wondering what's up with the JiHu label and its matching "gitlab-jh" group: https://about.gitlab.com/handbook/ceo/chief-of-staff-team/jihu-support/ https://about.gitlab.com/handbook/ceo/chief-of-staff-team/ji...
  - bloomburger 4y ago
    Why has the person who wrote this vuln, Zhu Shung, not made any commits/contributions since they pushed this out two months ago?: https://gitlab.com/memorycancel https://gitlab.com/memorycancel Can somebody who what they were trying to do here opine on if this might have been malicious or was more likely just a honest mistake?
  - xPaw 4y ago
    What I find mildly curious, that's also the only place where a length was provided as an argument into `Gitlab::Password.test_default`.
  - altcognito 4y ago
    I thought I had an account there for a while, I don't know how long (2-3 years?), and all of a sudden I got a password reset out of the blue regarding this particular change. No other emails in my history regarding gitlab whatsoever, so it's even possible I didn't have an account at all. I go over to the website try to login with my gmail account via SSO which fails because I "already have an account". So I proceed to reset password via email alone. After I'm in it tells me I've had an account since January 22nd 2022. Super unlikely i created the account this year, so I don't know what's going on over there, but it's not accurate bookkeeping.
  - fy20 4y ago
    Is there a better way to catch errors like this? Looking through the PR it looks like this file was accidentally changed, I assume with a project wide search and replace. I could easily imagine myself missing this when reviewing the PR "oh it's just changing a whole bunch of specs, go ahead".
    - NavinF 4y ago
      An integration test that creates dummy accounts using every method including SSO and then attempts to bruteforce the password should find “12345678” within an hour. I think a test like this would also have found the dropbox and macos bugs that let you login to any account by using an empty password: https://techcrunch.com/2011/06/20/dropbox-security-bug-made-passwords-optional-for-four-hours/ https://techcrunch.com/2011/06/20/dropbox-security-bug-made-... https://arstechnica.com/information-technology/2017/11/macos-bug-lets-you-log-in-as-admin-with-no-password-required/ https://arstechnica.com/information-technology/2017/11/macos... Edit: Oh, the password was "123qweQWE!@#000000000". Technically doable with an efficient password cracker that favors common patterns. zxcvbn’s entropy estimate says it will take 10^10.5 guesses. That’s 1 week at 50k/s. That’s a hell of an integration test for most software. https://lowe.github.io/tryzxcvbn/ https://lowe.github.io/tryzxcvbn/
      - dewey 4y ago
        > then attempts to bruteforce the password should find “12345678” within an hour But only if there's no rate limiting or increasing timeouts for wrong passwords which in most cases exists.
        NavinF 4y ago
        Right, but you can bypass that in a testing environment.
        dewey 4y ago
        Which then opens you up to exactly that class of bugs that also caused this issue. Having test specific code and feature flags and then testing a tweaked version isn't really covering all the cases then. Just like in this case where a hardcoded password was set to maybe log in through a test based on the naming "test_default".
  - theptip 4y ago
    Thanks for doing the issue sleuthing. This is an excruciatingly bad look. You'd have thought with all the code-owner functionality that GL has, they would lock down the `/lib/gitlab/auth/` files to require a security engineer to give additional signoff on top of a normal review. It looks like anyone at Gitlab can approve changes to the auth code (except LDAP): https://gitlab.com/gitlab-org/gitlab/-/blob/master/.gitlab/CODEOWNERS#L58 https://gitlab.com/gitlab-org/gitlab/-/blob/master/.gitlab/C... which is terrifying if true.
    - mdaniel 4y ago
      > to require a security engineer to give additional signoff on top of a normal review Like this? > cc @gitlab-com/gl-security/appsec https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#note_756311666 https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#... I do so desperately hope it doesn't come across as throwing shade, because hindsight-2020-etc, but I do also think there was some kind of weird process breakdown here because this change somehow slipped past a "4 eyes" and an appsec review phase
      - deleted 4y ago
        [deleted]
    - nightpool 4y ago
      This was already required in this case, you can see in the comments that an additional AppSec review was done and the appsec team signed off on the MR above and beyond the normal code review process.
      - bloomburger 4y ago
        And two of those appsec reviewers are now out of office for the next two weeks…
        orojackson 4y ago
        Heh, was this you [1]? Pretty much asking the same good questions you brought up in a different post. Additionally, I see that the Senior Director of Engineering, Tim Zallmann, has left a bunch of GitLab project repos about 14 hours ago as of this writing. He was one of the folks who tried pinging [3] Mr. Coutable (he's one of the reviewers that's currently OOO). The ping is likely regarding the discovery of the security vulnerability. [1] https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#note_898315619 https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#... [2] https://gitlab.com/users/timzallmann/activity https://gitlab.com/users/timzallmann/activity [3] https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#note_893221370 https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#...

krebsonsecurity 4y ago

This appears to be related. One Github user shared an alert they got today, two days after connecting their Github account to Gitlab. Something about an app added to the account. Their Github has 2fa turned on and a very strong password: https://twitter.com/briankrebs/status/1509910113716514822 https://twitter.com/briankrebs/status/1509910113716514822