Skip to content

Commit fbcb794

Browse files
KofTwentyTwoKofTwentyTwo
committed
fix(security): address CodeQL security warnings
- rapidoc: Add URL validation to prevent XSS via javascript: URLs - TableBasedAuthenticationModule: Upgrade from SHA1 to SHA256 with backward compatibility for legacy passwords - SecretsManagerUtils: Remove secretId from log messages to prevent info disclosure
1 parent d7c2065 commit fbcb794

File tree

4 files changed

+70
-22
lines changed

4 files changed

+70
-22
lines changed

‎qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/SecretsManagerUtils.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,8 @@ public static Optional<String> getSecret(String path, String name)
172172
}
173173
else
174174
{
175-
LOG.warn("SecretsManager secret at [" + secretId + "] was a JSON object, but it did not contain a key of [" + name + "] - so returning empty.");
175+
// Security: Don't log secretId as it reveals secret structure
176+
LOG.warn("SecretsManager secret was a JSON object, but it did not contain the expected key - returning empty.");
176177
return (Optional.empty());
177178
}
178179
}

‎qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/modules/authentication/implementations/TableBasedAuthenticationModule.java‎

Lines changed: 46 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -437,15 +437,20 @@ public static StateProviderInterface getStateProvider()
437437
*******************************************************************************/
438438
public static class PasswordHasher
439439
{
440-
private static final String PBKDF2_ALGORITHM = "PBKDF2WithHmacSHA1";
441-
private static final int SALT_BYTE_SIZE = 32;
442-
private static final int HASH_BYTE_SIZE = 32;
443-
private static final int PBKDF2_ITERATIONS = 1000;
440+
// Security: SHA256 is the current standard; SHA1 retained for legacy password validation
441+
private static final String PBKDF2_ALGORITHM_SHA256 = "PBKDF2WithHmacSHA256";
442+
private static final String PBKDF2_ALGORITHM_SHA1 = "PBKDF2WithHmacSHA1";
443+
private static final String ALGORITHM_PREFIX_SHA256 = "sha256";
444+
445+
private static final int SALT_BYTE_SIZE = 32;
446+
private static final int HASH_BYTE_SIZE = 32;
447+
private static final int PBKDF2_ITERATIONS = 100000; // OWASP recommended minimum for SHA256
444448

445449

446450

447451
/*******************************************************************************
448452
** Returns a salted, hashed version of a raw password.
453+
** Format: sha256:iterations:salt:hash (new) or iterations:salt:hash (legacy)
449454
**
450455
*******************************************************************************/
451456
public static String createHashedPassword(String password) throws NoSuchAlgorithmException, InvalidKeySpecException
@@ -460,24 +465,24 @@ public static String createHashedPassword(String password) throws NoSuchAlgorith
460465
///////////////////////
461466
// Hash the password //
462467
///////////////////////
463-
byte[] passwordHash = computePbkdf2Hash(password.toCharArray(), salt, PBKDF2_ITERATIONS, HASH_BYTE_SIZE);
468+
byte[] passwordHash = computePbkdf2Hash(password.toCharArray(), salt, PBKDF2_ITERATIONS, HASH_BYTE_SIZE, PBKDF2_ALGORITHM_SHA256);
464469

465-
//////////////////////////////////////////////////////
466-
// return string in the format iterations:salt:hash //
467-
//////////////////////////////////////////////////////
468-
return (PBKDF2_ITERATIONS + ":" + toHex(salt) + ":" + toHex(passwordHash));
470+
//////////////////////////////////////////////////////////////
471+
// return string in the format sha256:iterations:salt:hash //
472+
//////////////////////////////////////////////////////////////
473+
return (ALGORITHM_PREFIX_SHA256 + ":" + PBKDF2_ITERATIONS + ":" + toHex(salt) + ":" + toHex(passwordHash));
469474
}
470475

471476

472477

473478
/*******************************************************************************
474-
** Computes the PBKDF2 hash.
479+
** Computes the PBKDF2 hash using the specified algorithm.
475480
**
476481
*******************************************************************************/
477-
private static byte[] computePbkdf2Hash(char[] password, byte[] salt, int iterations, int bytes) throws NoSuchAlgorithmException, InvalidKeySpecException
482+
private static byte[] computePbkdf2Hash(char[] password, byte[] salt, int iterations, int bytes, String algorithm) throws NoSuchAlgorithmException, InvalidKeySpecException
478483
{
479484
PBEKeySpec spec = new PBEKeySpec(password, salt, iterations, bytes * 8);
480-
SecretKeyFactory skf = SecretKeyFactory.getInstance(PBKDF2_ALGORITHM);
485+
SecretKeyFactory skf = SecretKeyFactory.getInstance(algorithm);
481486

482487
return skf.generateSecret(spec).getEncoded();
483488
}
@@ -503,16 +508,40 @@ private static String toHex(byte[] array)
503508

504509
/*******************************************************************************
505510
** Validates a password against a hash.
511+
** Supports both new format (sha256:iterations:salt:hash) and legacy (iterations:salt:hash)
506512
**
507513
*******************************************************************************/
508514
private static boolean validatePassword(String password, String passwordHash) throws NoSuchAlgorithmException, InvalidKeySpecException
509515
{
510-
String[] params = passwordHash.split(":");
511-
int iterations = Integer.parseInt(params[0]);
512-
byte[] salt = fromHex(params[1]);
513-
byte[] hash = fromHex(params[2]);
516+
String[] params = passwordHash.split(":");
517+
String algorithm;
518+
int iterations;
519+
byte[] salt;
520+
byte[] hash;
521+
522+
///////////////////////////////////////////////////////////////////////////
523+
// Determine format: new format starts with algorithm prefix, legacy //
524+
// format starts with numeric iterations count //
525+
///////////////////////////////////////////////////////////////////////////
526+
if(params[0].equals(ALGORITHM_PREFIX_SHA256))
527+
{
528+
algorithm = PBKDF2_ALGORITHM_SHA256;
529+
iterations = Integer.parseInt(params[1]);
530+
salt = fromHex(params[2]);
531+
hash = fromHex(params[3]);
532+
}
533+
else
534+
{
535+
///////////////////////////////////////////////////////////////////////
536+
// Legacy format: iterations:salt:hash (uses SHA1 for compatibility) //
537+
///////////////////////////////////////////////////////////////////////
538+
algorithm = PBKDF2_ALGORITHM_SHA1;
539+
iterations = Integer.parseInt(params[0]);
540+
salt = fromHex(params[1]);
541+
hash = fromHex(params[2]);
542+
}
514543

515-
byte[] testHash = computePbkdf2Hash(password.toCharArray(), salt, iterations, hash.length);
544+
byte[] testHash = computePbkdf2Hash(password.toCharArray(), salt, iterations, hash.length, algorithm);
516545
return slowEquals(hash, testHash);
517546
}
518547

‎qqq-middleware-api/src/main/resources/rapidoc/rapidoc-container.html‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,14 +89,23 @@
8989
});
9090
});
9191

92+
function safeNavigate(url)
93+
{
94+
// Security: Only allow relative URLs starting with /
95+
if (url && url.startsWith('/'))
96+
{
97+
document.location.href = url;
98+
}
99+
}
100+
92101
function changeApi()
93102
{
94-
document.location.href = document.getElementById("otherApisSelect").value;
103+
safeNavigate(document.getElementById("otherApisSelect").value);
95104
}
96105

97106
function changeVersion()
98107
{
99-
document.location.href = document.getElementById("otherVersionsSelect").value;
108+
safeNavigate(document.getElementById("otherVersionsSelect").value);
100109
}
101110
</script>
102111
</body>

‎qqq-openapi/src/main/resources/rapidoc/rapidoc-container.html‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,14 +87,23 @@
8787
});
8888
});
8989

90+
function safeNavigate(url)
91+
{
92+
// Security: Only allow relative URLs starting with /
93+
if (url && url.startsWith('/'))
94+
{
95+
document.location.href = url;
96+
}
97+
}
98+
9099
function changeApi()
91100
{
92-
document.location.href = document.getElementById("otherApisSelect").value;
101+
safeNavigate(document.getElementById("otherApisSelect").value);
93102
}
94103

95104
function changeVersion()
96105
{
97-
document.location.href = document.getElementById("otherVersionsSelect").value;
106+
safeNavigate(document.getElementById("otherVersionsSelect").value);
98107
}
99108
</script>
100109
</body>

0 commit comments

Comments
 (0)