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

Java: QL Query Detector for JHipster Generated CVE-2019-16303 #4312

Merged
58 changes: 58 additions & 0 deletions java/ql/src/Security/CWE/CWE-338/JHipsterGeneratedPRNG.qhelp
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>This query detects instances of <code>RandomUtil.java</code> that were generated by a <a href="https://www.jhipster.tech/">JHipster</a> version that is vulnerable to <a href="https://github.com/jhipster/jhipster-kotlin/security/advisories/GHSA-j3rh-8vwq-wh84">CVE-2019-16303</a>.</p>

<p>If an app uses <code>RandomUtil.java</code> generated by a vulnerable version of JHipster, attackers can request a password reset token and use this to predict the value of future reset tokens generated by this server.
Using this information, they can create a reset link that allows them to take over any account.</p>

<p>This vulnerability has a
<a href="https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?name=CVE-2019-16303&amp;vector=AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H&amp;version=3.1&amp;source=NIST">
CVSS v3.0 Base Score of 9.8/10
</a>.</p>
</overview>

<example>

<p>The example below shows the vulnerable <code>RandomUtil</code> class generated by <a href="https://www.jhipster.tech/2019/09/13/jhipster-release-6.3.0.html">JHipster prior to version 6.3.0</a>.</p>
<sample src="JHipsterGeneratedPRNGVulnerable.java" />

<p>Below is a fixed version of the <code>RandomUtil</code> class.</p>
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved
<sample src="JHipsterGeneratedPRNGFixed.java" />
Comment on lines +20 to +23
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually use comments to highlight the GOOD and BAD code in examples. Is this possible here? If not, it might be worth highlighting the area that's different?

I'm not looking at a diff of the two code examples here, and I'm not a developer, so this may be a daft suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!


</example>

<recommendation>

<p>You should refactor the <code>RandomUtil</code> class and replace every call to <code>RandomStringUtils.randomAlphaNumeric</code>. You could regenerate the class using the latest version of JHipster, or use an automated refactoring. For example, using the <a href="https://github.com/moderneinc/jhipster-cwe-338">Patching JHipster CWE-338</a> for the <a href="https://github.com/openrewrite/rewrite">Rewrite project</a>.
</p>
</recommendation>

<references>

<li>
Cloudflare Blog:
<a href="https://blog.cloudflare.com/why-randomness-matters/">
Why secure systems require random numbers
</a>
</li>
<li>
Hacker News:
<a href="https://news.ycombinator.com/item?id=639976">
How I Hacked Hacker News (with arc security advisory)
</a>
</li>
<li>
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved
Posts by Pucara Information Security Team:
<a href="https://blog.pucarasec.com/2020/05/09/the-java-soothsayer-a-practical-application-for-insecure-randomness-includes-free-0day/">
The Java Soothsayer: A practical application for insecure randomness. (Includes free 0day)
</a>
</li>

<!-- LocalWords: CWE random RNG PRNG CSPRNG SecureRandom JHipster -->

</references>

</qhelp>
50 changes: 50 additions & 0 deletions java/ql/src/Security/CWE/CWE-338/JHipsterGeneratedPRNG.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/**
* @name Detect JHipster Generator Vulnerability CVE-2019-16303
* @description Using a vulnerable version of JHipster to generate random numbers makes it easier for attackers to take over accounts.
* @kind problem
* @problem.severity error
* @precision very-high
* @id java/jhipster-prng
* @tags security
* external/cwe/cwe-338
*/

import java
import semmle.code.java.frameworks.apache.Lang

private class PredictableApacheRandomStringUtilsMethod extends Method {
PredictableApacheRandomStringUtilsMethod() {
this.getDeclaringType() instanceof TypeApacheRandomStringUtils and
// The one valid use of this type that uses SecureRandom as a source of data.
not this.getName() = "random"
}
}

private class PredictableApacheRandomStringUtilsMethodAccess extends MethodAccess {
PredictableApacheRandomStringUtilsMethodAccess() {
this.getMethod() instanceof PredictableApacheRandomStringUtilsMethod
}
}

private class VulnerableJHipsterRandomUtilClass extends Class {
VulnerableJHipsterRandomUtilClass() {
// The package name that JHipster generated the 'RandomUtil' class in was dynamic. Thus 'hasQualifiedName' can not be used here.
getName() = "RandomUtil"
}
}

private class VulnerableJHipsterRandomUtilMethod extends Method {
VulnerableJHipsterRandomUtilMethod() {
this.getDeclaringType() instanceof VulnerableJHipsterRandomUtilClass and
this.getName().matches("generate%") and
this.getReturnType() instanceof TypeString and
exists(ReturnStmt s |
s = this.getBody().(SingletonBlock).getStmt() and
s.getResult() instanceof PredictableApacheRandomStringUtilsMethodAccess
)
}
}

from VulnerableJHipsterRandomUtilMethod method
select method,
"Weak random number generator used in security sensitive method (JHipster CVE-2019-16303)."
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import org.apache.commons.lang3.RandomStringUtils;

import java.security.SecureRandom;

/**
* Utility class for generating random Strings.
*/
public final class RandomUtil {
private static final SecureRandom SECURE_RANDOM = new SecureRandom(); // GOOD: Using SecureRandom

private static final int DEF_COUNT = 20;

static {
SECURE_RANDOM.nextBytes(new byte[64]);
}

private RandomUtil() {
}

private static String generateRandomAlphanumericString() {
// GOOD: Passing Secure Random to RandomStringUtils::random
return RandomStringUtils.random(DEF_COUNT, 0, 0, true, true, null, SECURE_RANDOM);
}

/**
* Generate a password.
*
* @return the generated password.
*/
public static String generatePassword() {
return generateRandomAlphanumericString();
}

/**
* Generate an activation key.
*
* @return the generated activation key.
*/
public static String generateActivationKey() {
return generateRandomAlphanumericString();
}

/**
* Generate a reset key.
*
* @return the generated reset key.
*/
public static String generateResetKey() {
return generateRandomAlphanumericString();
}

/**
* 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 generateRandomAlphanumericString();
}

/**
* Generate a persistent token, used in the authentication remember-me mechanism.
*
* @return the generated token data.
*/
public static String generateTokenData() {
return generateRandomAlphanumericString();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import org.apache.commons.lang3.RandomStringUtils;

/**
* Utility class for generating random Strings.
*/
public final class RandomUtil {

private static final int DEF_COUNT = 20;

private RandomUtil() {
}

/**
* Generate a password.
*
* @return the generated password.
*/
public static String generatePassword() {
return RandomStringUtils.randomAlphanumeric(DEF_COUNT); // BAD: RandomStringUtils does not use SecureRandom
}

/**
* Generate an activation key.
*
* @return the generated activation key.
*/
public static String generateActivationKey() {
return RandomStringUtils.randomNumeric(DEF_COUNT); // BAD: RandomStringUtils does not use SecureRandom
}

/**
* Generate a reset key.
*
* @return the generated reset key.
*/
public static String generateResetKey() {
return RandomStringUtils.randomNumeric(DEF_COUNT); // BAD: RandomStringUtils does not use SecureRandom
}

/**
* 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); // BAD: RandomStringUtils does not use SecureRandom
}

/**
* 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); // BAD: RandomStringUtils does not use SecureRandom
}
}
12 changes: 12 additions & 0 deletions java/ql/src/semmle/code/java/frameworks/apache/Lang.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/** Definitions related to the Apache Commons Lang library. */

import java

/*--- Types ---*/
/** The class `org.apache.commons.lang.RandomStringUtils` or `org.apache.commons.lang3.RandomStringUtils`. */
class TypeApacheRandomStringUtils extends Class {
TypeApacheRandomStringUtils() {
hasQualifiedName("org.apache.commons.lang", "RandomStringUtils") or
hasQualifiedName("org.apache.commons.lang3", "RandomStringUtils")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
| vulnerable/RandomUtil.java:20:26:20:41 | generatePassword | Weak random number generator used in security sensitive method (JHipster CVE-2019-16303). |
| vulnerable/RandomUtil.java:29:26:29:46 | generateActivationKey | Weak random number generator used in security sensitive method (JHipster CVE-2019-16303). |
| vulnerable/RandomUtil.java:38:26:38:41 | generateResetKey | Weak random number generator used in security sensitive method (JHipster CVE-2019-16303). |
| vulnerable/RandomUtil.java:48:26:48:43 | generateSeriesData | Weak random number generator used in security sensitive method (JHipster CVE-2019-16303). |
| vulnerable/RandomUtil.java:57:26:57:42 | generateTokenData | Weak random number generator used in security sensitive method (JHipster CVE-2019-16303). |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Security/CWE/CWE-338/JHipsterGeneratedPRNG.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package test.cwe338.cwe.examples.fixed;

import org.apache.commons.lang3.RandomStringUtils;

import java.security.SecureRandom;

/**
* Utility class for generating random Strings.
*/
public final class RandomUtil {
private static final SecureRandom SECURE_RANDOM = new SecureRandom();

private static final int DEF_COUNT = 20;

static {
SECURE_RANDOM.nextBytes(new byte[64]);
}

private RandomUtil() {
}

private static String generateRandomAlphanumericString() {
return RandomStringUtils.random(DEF_COUNT, 0, 0, true, true, null, SECURE_RANDOM);
}

/**
* Generate a password.
*
* @return the generated password.
*/
public static String generatePassword() {
return generateRandomAlphanumericString();
}

/**
* Generate an activation key.
*
* @return the generated activation key.
*/
public static String generateActivationKey() {
return generateRandomAlphanumericString();
}

/**
* Generate a reset key.
*
* @return the generated reset key.
*/
public static String generateResetKey() {
return generateRandomAlphanumericString();
}

/**
* 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 generateRandomAlphanumericString();
}

/**
* Generate a persistent token, used in the authentication remember-me mechanism.
*
* @return the generated token data.
*/
public static String generateTokenData() {
return generateRandomAlphanumericString();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../../stubs/apache-commons-lang3-3.7
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package test.cwe338.cwe.examples.vulnerable;

import org.apache.commons.lang3.RandomStringUtils;

/**
* Utility class for generating random Strings.
*/
public final class RandomUtil {

private static final int DEF_COUNT = 20;

private RandomUtil() {
}

/**
* 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);
}

/**
* 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);
}
}
Loading