Skip to content

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

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

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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>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>

<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.
<sample src="JHipsterGeneratedPRNGVulnerble.java" />

<p>Below is a fixed version of the <code>RandomUtil</code> class.</p>
<sample src="JHipsterGeneratedPRNGFixed.java" />

</example>

<recommendation>

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

<!-- 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
* @description Detector for the CVE-2019-16303 vulnerability that existed in the JHipster code generator.
* @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"
}
}

private class VulnerableJHipsterRandomUtilClass extends Class {
VulnerableJHipsterRandomUtilClass() { 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, PredictableApacheRandomStringUtilsMethodAccess access |
s = this.getBody().(SingletonBlock).getStmt()
|
s.getResult() = access
)
}
}

from VulnerableJHipsterRandomUtilMethod the_method
select the_method,
"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);
}
}
9 changes: 9 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,9 @@
/* Definitions related to the Apache Commons Exec library. */
import semmle.code.java.Type

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