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
55 changes: 55 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,55 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>This query detects instances of <code>RandomUtil.java</code> generated by a <a href="https://www.jhipster.tech/">JHipster</a> version vulnerable to <a href="https://github.com/jhipster/jhipster-kotlin/security/advisories/GHSA-j3rh-8vwq-wh84">CVE-2019-16303</a>.</p>
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved

<p>Using one password reset token from your app combined with the proof of concept (POC) linked below, an attacker can determine 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.</p>
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved

<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 JHipster.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Presumably this is what vulnerable versions of JHipster generate? If so, we should mention 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.

Can do

<sample src="JHipsterGeneratedPRNGVulnerble.java" />
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved

<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" />

</example>

<recommendation>

<p>An automated refactoring <a href="https://github.com/openrewrite/rewrite">rewrite</a> module <a href="https://github.com/moderneinc/jhipster-cwe-338"> can be found here</a>.</p>
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved
</recommendation>

<references>

<li>
<a href="https://blog.cloudflare.com/why-randomness-matters/">
Cloudflare Blog: Why secure systems require random numbers
</a>
</li>
<li>
<a href="https://news.ycombinator.com/item?id=639976">
How I Hacked Hacker News (with arc security advisory)
</a>
</li>
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved
<li>
Research (Hacking Apache Commons RandomStringUtils):
<a href="https://web.archive.org/web/20191126104359/https://medium.com/@alex91ar/the-java-soothsayer-a-practical-application-for-insecure-randomness-c67b0cd148cd">
The Java Soothsayer: A practical application for insecure randomness. (Includes free 0day)
</a>
</li>
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved

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

</references>

</qhelp>
48 changes: 48 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,48 @@
/**
* @name Detect JHipster Generator Vulnnerability CVE-2019-16303
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved
* @description Detector for the CVE-2019-16303 vulnerability that existed in the JHipster code generator.
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved
* @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
}
}

private class PredictableApacheRandomStringUtilsMethodAccess extends MethodAccess {
PredictableApacheRandomStringUtilsMethodAccess() {
this.getMethod() instanceof PredictableApacheRandomStringUtilsMethod and
// The one valid use of this type that uses SecureRandom as a source of data.
not this.getMethod().getName() = "random"
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved
}
}

private class VulnerableJHipsterRandomUtilClass extends Class {
VulnerableJHipsterRandomUtilClass() { getName() = "RandomUtil" }
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved
}

private class VulnerableJHipsterRandomUtilMethod extends Method {
VulnerableJHipsterRandomUtilMethod() {
this.getDeclaringType() instanceof VulnerableJHipsterRandomUtilClass and
this.getName().matches("generate%") and
this.getReturnType() instanceof TypeString and
exists(ReturnStmt s, PredictableApacheRandomStringUtilsMethodAccess access |
s = this.getBody().(SingletonBlock).getStmt()
|
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved
s.getResult() = access
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved
)
}
}

from VulnerableJHipsterRandomUtilMethod the_method
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved
select the_method,
aibaars marked this conversation as resolved.
Show resolved Hide resolved
"RandomUtil was generated by JHipster Generator version vulnerable to CVE-2019-16303"
Copy link
Contributor

Choose a reason for hiding this comment

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

This query catches cases of badly generated RandomUtil classes. However, it would be possible to generalise this query even further. If I am not mistaken the actual problem is the use of java.util.Random (or wrapper classes like RandomStringUtils) in places where you're doing something crypographically sensitive.

It is of course very hard to write a query that finds all "crypographically sensitive" pieces of code. However, we could start by treating methods named generatePassword , generate%Token%, generate%Key, etc. as likely to be sensitive. That way the query should flag up the generated JHipster classes and potentially many more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aibaars and I chatted about this privately. I have another query I'm working on that is more general here:
#2694

Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
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,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);
}

/**
* 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);
}
}
11 changes: 11 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,11 @@
/* Definitions related to the Apache Commons Lang library. */
JLLeitschuh marked this conversation as resolved.
Show resolved Hide resolved
import semmle.code.java.Type

/*--- 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 | RandomUtil was generated by JHipster Generator version vulnerable to CVE-2019-16303 |
| vulnerable/RandomUtil.java:29:26:29:46 | generateActivationKey | RandomUtil was generated by JHipster Generator version vulnerable to CVE-2019-16303 |
| vulnerable/RandomUtil.java:38:26:38:41 | generateResetKey | RandomUtil was generated by JHipster Generator version vulnerable to CVE-2019-16303 |
| vulnerable/RandomUtil.java:48:26:48:43 | generateSeriesData | RandomUtil was generated by JHipster Generator version vulnerable to CVE-2019-16303 |
| vulnerable/RandomUtil.java:57:26:57:42 | generateTokenData | RandomUtil was generated by JHipster Generator version vulnerable to 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