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

Bug bounty for security advisory - thank you @JLLeitschuh #10401

Closed
jdubois opened this issue Sep 13, 2019 · 48 comments
Closed

Bug bounty for security advisory - thank you @JLLeitschuh #10401

jdubois opened this issue Sep 13, 2019 · 48 comments
Labels
$$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ $500 https://www.jhipster.tech/bug-bounties/
Milestone

Comments

@jdubois
Copy link
Member

jdubois commented Sep 13, 2019

This is for GHSA-mwp6-j9wf-968c which isn't public at the time of this writing

@JLLeitschuh thank you so much for reporting this issue!!!

We would like to thank you by giving you a $500 bug bounty on the project, and this is why I'm creating this ticket (this is to follow our official process to give money).

To have more information on our bug bounties program, please read https://www.jhipster.tech/bug-bounties/

@jdubois jdubois added $$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ $500 https://www.jhipster.tech/bug-bounties/ labels Sep 13, 2019
@jdubois jdubois changed the title Bug bounty for security advisory Bug bounty for security advisory - thank you @JLLeitschuh Sep 13, 2019
@jdubois jdubois added this to the 6.3.0 milestone Sep 13, 2019
@JLLeitschuh
Copy link
Contributor

Wow! Thank you! This is incredibly kind of you guys!

@atomfrede
Copy link
Member

You are welcome! Thanks for your advice! If you have any questions regarding the bounty process don't hesitate to ask 👍

@JLLeitschuh
Copy link
Contributor

JLLeitschuh commented Sep 13, 2019

@jdubois You should have finished filling out that disclosure form before submitting it. The disclosure in it's current format, unfortunately, isn't that useful to anyone. Plus there currently any CVE numbers issued for this vulnerability.

I will be moving forward with the CVE number request.

Here is the full disclosure of this vulnerability


CWE-338: Use of Cryptographically Weak Pseudo-Random Number Generator (PRNG)

JHipster is using an insecure source of randomness to generate all of it's random values. JHipster relies upon apache commons lang3 RandomStringUtils.

From the documentation:

Caveat: Instances of Random, upon which the implementation of this class relies, are not cryptographically secure.
- https://commons.apache.org/proper/commons-lang/javadocs/api-3.9/org/apache/commons/lang3/RandomStringUtils.html

Here are the examples of JHipster's use of an insecure PRNG:

/**
* Generate a password.
*
* @return the generated password.
*/
public static String generatePassword() {
return RandomStringUtils.randomAlphanumeric(DEF_COUNT);
}
/**
* Generate an activation key.
*
* @return the generated activation key.
*/
public static String generateActivationKey() {
return RandomStringUtils.randomNumeric(DEF_COUNT);
}
/**
* Generate a reset key.
*
* @return the generated reset key.
*/
public static String generateResetKey() {
return RandomStringUtils.randomNumeric(DEF_COUNT);
}
<%_ if (authenticationType === 'session' && !reactive) { _%>
/**
* Generate a unique series to validate a persistent token, used in the
* authentication remember-me mechanism.
*
* @return the generated series data.
*/
public static String generateSeriesData() {
return RandomStringUtils.randomAlphanumeric(DEF_COUNT);
}
/**
* Generate a persistent token, used in the authentication remember-me mechanism.
*
* @return the generated token data.
*/
public static String generateTokenData() {
return RandomStringUtils.randomAlphanumeric(DEF_COUNT);
}

Proof Of Concepts Already Exist

There has been a POC of taking one RNG value generated RandomStringUtils and reversing it to generate all of the past/future RNG values public since March 3rd, 2018.

https://medium.com/@alex91ar/the-java-soothsayer-a-practical-application-for-insecure-randomness-c67b0cd148cd

POC Repository: https://github.com/alex91ar/randomstringutils

Potential Impact Technical

All that is required is to get one password reset token from a JHipster generated service and using the POC above, you can reverse what all future password reset tokens to be generated by this server. This allows an attacker to pick and choose what account they would like to takeover by sending account password reset requests for targeted accounts.

Potential Impact Scale

You have a lot of companies using JHipster:
https://www.jhipster.tech/companies-using-jhipster/

You have ~15k projects that all contain the RandomUtil class code generated by JHipster:
https://github.com/search?l=Java&p=2&q=Utility+class+for+generating+random+Strings.+RandomUtil&type=Code

You have 26k projects using your project according to GitHub's dependency insights:
https://github.com/jhipster/generator-jhipster/network/dependents?package_id=UGFja2FnZS0xODI1MDExMA%3D%3D


The release notes are here:

https://www.jhipster.tech/2019/09/13/jhipster-release-6.3.0.html

@jdubois
Copy link
Member Author

jdubois commented Sep 13, 2019

I'm sorry @JLLeitschuh which form are you talking about?

@JLLeitschuh
Copy link
Contributor

@jdubois The form that generated this advisory:

GHSA-mwp6-j9wf-968c

You had to expand the advisory to fill in the forms to generate it. Also, you should have filed for a CVE before disclosure of this vulnerability.

I'm working on getting that issued right now.

@jdubois
Copy link
Member Author

jdubois commented Sep 13, 2019

Oh I totally missed you could click on the title!!! I thought it was just some fancy formatting!
Thank you for taking care of this.

@JLLeitschuh
Copy link
Contributor

This one:

Screen Shot 2019-09-13 at 6 08 13 PM

@JLLeitschuh
Copy link
Contributor

@jdubois Can you please contact GitHub support to get a fully written up public disclosure replaced with the current contents? I will get the CVE number for that disclosure.

@jdubois
Copy link
Member Author

jdubois commented Sep 13, 2019

Oh I missed this link too!!! I'm lost here. Is this just me?? I'll contact Github support, but probably after the weekend.

@JLLeitschuh
Copy link
Contributor

JLLeitschuh commented Sep 13, 2019

@jdubois This is time-sensitive. This vulnerability is now public and the full details are now publicly disclosed & is trivial to exploit. We need to move more quickly than waiting till Monday to alert the public. Please contact (or have someone from your team) ASAP.

@JLLeitschuh
Copy link
Contributor

The full attack scenario is detailed as follows:

So let me see if I got it, an attack scenario would be:

  1. attacker goes to password reset page and enters an email address he/she owns to receive an initial token
  2. attacker goes to password reset page and enters email address of a potential user of this app
  3. attacker generates a token and goes to confirmation page and enters a new password

This would work as long as the app did not generate another random string between steps 1 and 2, which is probable. But even if the first fake token would not work, attacker could try next one as we don't have anything to limit the number of attempts to finish password reset.

The attack is not silent though as the victim would receive a password reset mail.
- @gmarziou

This attack scenario is completely valid and would work against anyone with these servers deployed today.

@jdubois
Copy link
Member Author

jdubois commented Sep 13, 2019

I agree, but the fix has been published, as well as a clear explanation at the top of the release notes, explaining what to do.

@jdubois
Copy link
Member Author

jdubois commented Sep 13, 2019

We also have dependabot sending patches, which seem to have the correct information.
All that is wrong is the advisory text, and I admit doing a mistake here, but now it is read-only.
I'll add a comment and notify the person from Github who was ready to help.
I'm sorry but I won't have much time over the weekend.

@JLLeitschuh
Copy link
Contributor

A CVE number has been requested. I'll follow up when MITRE has gotten back to me.

@JLLeitschuh
Copy link
Contributor

@jdubois Can you prioritize getting this fixed as well?

jhipster/jhipster-kotlin#183

@jdubois
Copy link
Member Author

jdubois commented Sep 13, 2019

@rschultheis could you help us here? I published the security advisory without filling it up, and now it's read-only. Could we edit it?

@JLLeitschuh
Copy link
Contributor

JLLeitschuh commented Sep 13, 2019

@jdubois Expense submitted:
https://opencollective.com/generator-jhipster/expenses/10064

Thank you very much for the bounty! I really appreciate it! $500 more towards my college loans. 😄

@JLLeitschuh
Copy link
Contributor

CVE has been assigned. (The link will be live shortly):

https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-16303

@jdubois
Copy link
Member Author

jdubois commented Sep 14, 2019

Ok, as far as I understand, the only issue we have is the text of the advisory which is wrong - that's my fault and I hope we can edit it.
For all our users, our usual update process is in place, and also those using dependabot have received updates (that's also something we need to improve: everybody should use dependabot).
The release notes are here: https://www.jhipster.tech/2019/09/13/jhipster-release-6.3.0.html

Thanks a lot @JLLeitschuh !

@atomfrede
Copy link
Member

@jhipster/developers can someone with merge rights take a look at jhipster kotlin?

@pascalgrimaud
Copy link
Member

Bounty just approuved. Thanks a lot @JLLeitschuh for your help !

@atomfrede : I'm having a look

@pascalgrimaud
Copy link
Member

@atomfrede : same question here, can this ticket be closed or is it not finished yet ?

@jdubois
Copy link
Member Author

jdubois commented Sep 16, 2019

Unfortunately we haven't been able to update the vulnerability report - I'm hoping we can get help from @rschultheis so let's keep this opened a few days, and close it if we have no solution.
We definitely need to work on that process so we don't have other issues next time. Also, I'll try to contact people at GitHub (I work in the same company now!) to talk about UI/UX, and also about using all this from a mobile phone - I think one of the issues is that it's really hard to do all this with a mobile phone, when that's one of my main tools for JHipster.

@JLLeitschuh
Copy link
Contributor

@jdubois As I suspected it would, this vulnerability was given a CVSSv3 score of 9.8/10

https://nvd.nist.gov/vuln/detail/CVE-2019-16303

@JLLeitschuh
Copy link
Contributor

@jdubois Your organization has an email address for every single company that has publicly stated that they are using JHipster as indicated by your form.

Screen Shot 2019-09-16 at 11 41 31 AM

I advise that you take advantage of having this list of contacts and reach out to them directly about this vulnerability so that they can get this issue patched.
Feel free to CC me into that email chain (email below).

As an aside: if any of these companies have their own BB programs you can tell them I accept donations to my PayPal account at Jonathan.Leitschuh@gmail.com. By no means a requirement, but every bit towards my college loans helps.

@jdubois
Copy link
Member Author

jdubois commented Sep 16, 2019

Indeed, I also have the emails of more than 30,000 users who registered to https://start.jhipster.tech

Then, none of those persons have given us the right to mass-email them, even for their own good, so I'm not sure what is the right thing to do here

@JLLeitschuh
Copy link
Contributor

Then, none of those persons have given us the right to mass-email them, even for their own good, so I'm not sure what is the right thing to do here

I have a friend with some legal experience in this space that I've reached out to for their opinion on this.

You could also try asking the EFF.

@rschultheis
Copy link

👋 @jdubois

@rschultheis could you help us here? I published the security advisory without filling it up, and now it's read-only. Could we edit it?

Sorry just getting caught up with some notifications, and we can defnitely do something to help here.

I did indeed notice that advisory get submitted with the default template. I the UI there is a bit confusing and we are taking a look at how to make this problem harder to happen.

But more importantly, our product doesn't offer update yet! I'll discuss with the team, is there a preferred summary and description you would live to have in the advisory @jdubois ?

Our GitHub security advisory is currently published with this as the description:

Account takeover and privilege escalation is possible in applications generated by generator-jhipster before 6.3.0. This is due to a vulnerability in the generated java classes: CWE-338: Use of Cryptographically Weak Pseudo-Random Number Generator (PRNG)

Generated applications must be manually patched, following instructions in the release notes: https://www.jhipster.tech/2019/09/13/jhipster-release-6.3.0.html

@jdubois
Copy link
Member Author

jdubois commented Sep 18, 2019

Thanks a lot @rschultheis !!

  • The title and description you propose are correct. It's the rest of the form ( @JLLeitschuh can you confirm??) that can be seen at the bottom of GHSA-mwp6-j9wf-968c which has the template data, instead of real instructions.
  • I will contact you in parallel about this through Microsoft :-)

@rschultheis
Copy link

OK @jdubois I've updated the advisory with description I proposed, as well as additional meta data like the CVE ID. Let me know what you think!

I am more than happy to make additional changes to the description if you would like!

Also, we clearly need to support updating advisories after publish, thanks for pinging me on this issue here!

@SudharakaP
Copy link
Member

@jdubois : If I may chip in with a minor suggestion; I noticed that the security policy has your email address. I would suggest maybe add an address which goes to all the team leads? Just had this thought, since this way if you aren't available someone else can look into it. 😄

@JLLeitschuh
Copy link
Contributor

Also, consider moving your security policy to a .github repository so that is applied to the entire organization.

@PierreBesson
Copy link
Contributor

PierreBesson commented Sep 19, 2019

It's already created : https://github.com/jhipster/.github
For the dedicated security email we don't have one currently'

@jdubois
Copy link
Member Author

jdubois commented Sep 19, 2019

Thanks everyone for the help!!!

  • @rschultheis yes that's good!
  • @SudharakaP thanks for notifying me of my email address, I know this is an issue. The problem is that I bought the "jhipster.tech" domain name when the project had no money, so I took the cheapest option possible, and I have seen that those emails are sometimes lost.... And as I want to be sure not to miss them, I'd rather put my email address, at least for the moment. I have the same issue with our CoC, so I need to find a way to do some private and reliable mailing lists.

@ruddell
Copy link
Member

ruddell commented Sep 19, 2019

Related to my comment here, I noticed the release notes (and the advisory tweet) state that you are only affected if you use JWT, but doesn't this also affect Session and UAA auth types? They both use RandomUtil for generating reset tokens.

@JLLeitschuh
Copy link
Contributor

@ruddell These components are also impacted and vulnerable.

@jdubois
Copy link
Member Author

jdubois commented Sep 19, 2019

Oh that's an awesome catch @ruddell - I feel ashamed by this, especially as I coded the session authentication, so I should be the best to know!!
Session authentication & UAA are very rarely used now (we have the stats in https://start.jhipster.tech/#/statistics but we haven't done the graphs, so we could have the real numbers), and to be honest I haven't used any of them for a long, long time. That's why I forgot about them. In fact, I'm thinking we should deprecate them, but that's another issue.

So yes, those are also vulnerable: I can update the release notes, and warn people in tomorrow's release. It's a bit strange because this is already fixed, but that's probably the only solution.

@JLLeitschuh
Copy link
Contributor

@jdubois RE: Emailing discussed above 👆 #10401 (comment)

The advice from US CERT was basically, they are not lawyers and so they can't provide legal advice. That being said:

I would encourage JHipster to notify as many of their customers as they fell comfortable with.

I think from an ethical point of view, it's the right thing to do and you should send out an email disclosing this vulnerability to all potentially impacted parties.

If you move forward with this though, please make sure that you BCC your email so that everyone can't see everyone else's address. You may want to reach out to Microsoft's security team for assistance on this one.

@jdubois
Copy link
Member Author

jdubois commented Sep 19, 2019

Thanks @JLLeitschuh I'll have a look at what we can do - I have no mass email solution to send emails to 30,000 people. Our contact channel as always been Twitter - I know it's not perfect, but that's easy and free.
I'm going to have a look at a specific mailing list solution for this, to which people can subscribe.

@SudharakaP
Copy link
Member

SudharakaP commented Sep 19, 2019

@jdubois : Seems like you guys already use Google groups; which means you should have a Google Admin account. Suggestion; you could probably leverage this and create a custom Google group with invite only access to external members with appropriate access privileges and roles. 😄

image

@jdubois
Copy link
Member Author

jdubois commented Sep 20, 2019

@SudharakaP oh yes but it's really a mess. The UI sucks, it's awful to manage (I'm going to use it today again, I never know where to click). Also, I need to check how this works: in fact I realize this shouldn't be a mailing list, it's more an annoucement list. People shouldn't be able to send mails to each other, otherwise it's going to be a mess.

@SudharakaP
Copy link
Member

SudharakaP commented Sep 20, 2019

@jdubois : Yes I agree, the UI is not quite obvious. 😄 But I think this fits the bill if you could get the permissions set up correctly.

No they cannot see or reply to the whole list; notice there's a setting on the following page; called "Post Replies". I haven't tested this though; you probably need to run some tests. 😄

image

@SudharakaP
Copy link
Member

SudharakaP commented Sep 20, 2019

@jdubois : So I've went ahead and tested this out for you. Here are the preliminary steps. With these each user will receive emails when you post to the distribution list. They can only reply to the owner. If they try to reply to the group it will bounce back. Let me know if you need any further help. I can do some more tests if you'd like. 😄 😄

  1. Create the group with maximum restrictive privileges but with "Only invited users" and "Allow Members Outside Group". You can make it even simpler if you just remove all managers and just keep owners.

image

  1. Go to "Advanced Settings" and except for the owner(s)/manager(s); select all other members and do a bulk "Disallow Posting"

image

  1. Go to the "Email Options" and change the "Post Replies" option (Suggestion: "Owners Only" ?)

image

  1. Go to "Basic Permissions" and make sure everything is "Owners Only".

image

  1. Both Moderation Permission and Access Permissions set everything to Owner/Managers Only.

  2. Posting Permissions; do not know whether this has any effect or not; but for added safety make "Reply to Author" appropriately set.

image

  1. Posting Options should be set to "Email Only"

image

All this being said; I should say it might be easier and elegant to purchase a bulk email solution if you guys have the budget. 😄 😄

@jdubois
Copy link
Member Author

jdubois commented Sep 20, 2019

Thanks so much!!! I'll have a close look at this.

@atomfrede
Copy link
Member

I think we can close this and discuss further things on our mailing list if needed.

@JLLeitschuh
Copy link
Contributor

Hey @jdubois,

Now that GitHub supports giving credit on disclosures, would you be willing to put me as the finder on both of these?

GHSA-mwp6-j9wf-968c
GHSA-j3rh-8vwq-wh84

Also, the Daily Swig covered this, and they kinda got the story wrong, so I'm working with them to correct it.
https://portswigger.net/daily-swig/app-generator-tool-jhipster-kotlin-fixes-fundamental-cryptographic-bug

@atomfrede
Copy link
Member

@JLLeitschuh I have updated the Kotlin one, I can't update the one in the main generator.

@jdubois
Copy link
Member Author

jdubois commented Jul 14, 2020

Hi @ JLLeitschuh it should be OK for both issues! Thank you for everything

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
$$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ $500 https://www.jhipster.tech/bug-bounties/
Projects
None yet
Development

No branches or pull requests

8 participants