Skip to content

Commit f48c418

Browse files
smowtonsmowton
authored
Merge pull request github#5907 from x-f1v3/java/hardcoded-shiro-key
Java: CWE-798: Query to detect hard-coded SHIRO key
2 parents 60a023d + ec4cb7c commit f48c418

File tree

17 files changed

+510
-5
lines changed

17 files changed

+510
-5
lines changed

‎java/change-notes/2021-05-24-hardcoded-shiro-key-in-api-call.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
lgtm,codescanning
2+
* The query "Hard-coded credential in API call" (`java/hardcoded-credential-api-call`) can now detect a hard-coded Apache Shiro cipher key.
3+
* The query "Hard-coded credential in API call" (`java/hardcoded-credential-api-call`) now detects hard-coded credentials that are Base64 encoded or decoded before use.

‎java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,8 @@ private predicate summaryModelCsv(string row) {
309309
"java.util;Base64$Decoder;false;decode;(ByteBuffer);;Argument[0];ReturnValue;taint",
310310
"java.util;Base64$Decoder;false;decode;(String);;Argument[0];ReturnValue;taint",
311311
"java.util;Base64$Decoder;false;wrap;(InputStream);;Argument[0];ReturnValue;taint",
312+
"cn.hutool.core.codec;Base64;true;decode;;;Argument[0];ReturnValue;taint",
313+
"org.apache.shiro.codec;Base64;false;decode;(String);;Argument[0];ReturnValue;taint",
312314
"org.apache.commons.codec;Encoder;true;encode;(Object);;Argument[0];ReturnValue;taint",
313315
"org.apache.commons.codec;Decoder;true;decode;(Object);;Argument[0];ReturnValue;taint",
314316
"org.apache.commons.codec;BinaryEncoder;true;encode;(byte[]);;Argument[0];ReturnValue;taint",

‎java/ql/src/Security/CWE/CWE-798/HardcodedCredentialsApiCall.ql

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,30 @@ class HardcodedCredentialApiCallConfiguration extends DataFlow::Configuration {
2727

2828
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
2929
node1.asExpr().getType() instanceof TypeString and
30-
exists(MethodAccess ma | ma.getMethod().hasName(["getBytes", "toCharArray"]) |
31-
node2.asExpr() = ma and
32-
ma.getQualifier() = node1.asExpr()
30+
(
31+
exists(MethodAccess ma | ma.getMethod().hasName(["getBytes", "toCharArray"]) |
32+
node2.asExpr() = ma and
33+
ma.getQualifier() = node1.asExpr()
34+
)
35+
or
36+
// These base64 routines are usually taint propagators, and this is not a general
37+
// TaintTracking::Configuration, so we must specifically include them here
38+
// as a common transform applied to a constant before passing to a remote API.
39+
exists(MethodAccess ma |
40+
ma.getMethod()
41+
.hasQualifiedName([
42+
"java.util", "cn.hutool.core.codec", "org.apache.shiro.codec",
43+
"apache.commons.codec.binary", "org.springframework.util"
44+
], ["Base64$Encoder", "Base64$Decoder", "Base64", "Base64Utils"],
45+
[
46+
"encode", "encodeToString", "decode", "decodeBase64", "encodeBase64",
47+
"encodeBase64Chunked", "encodeBase64String", "encodeBase64URLSafe",
48+
"encodeBase64URLSafeString"
49+
])
50+
|
51+
node1.asExpr() = ma.getArgument(0) and
52+
node2.asExpr() = ma
53+
)
3354
)
3455
}
3556

‎java/ql/src/Security/CWE/CWE-798/SensitiveApi.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,5 +513,6 @@ private predicate otherApiCallableCredentialParam(string s) {
513513
s = "com.amazonaws.auth.BasicAWSCredentials;BasicAWSCredentials(String, String);1" or
514514
s = "com.azure.identity.UsernamePasswordCredentialBuilder;username(String);0" or
515515
s = "com.azure.identity.UsernamePasswordCredentialBuilder;password(String);0" or
516-
s = "com.azure.identity.ClientSecretCredentialBuilder;clientSecret(String);0"
516+
s = "com.azure.identity.ClientSecretCredentialBuilder;clientSecret(String);0" or
517+
s = "org.apache.shiro.mgt.AbstractRememberMeManager;setCipherKey(byte[]);0"
517518
}

‎java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCredentialsApiCall.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ edges
2626
| HardcodedAzureCredentials.java:61:3:61:33 | new HardcodedAzureCredentials(...) [clientSecret] : String | HardcodedAzureCredentials.java:15:14:15:42 | parameter this [clientSecret] : String |
2727
| HardcodedAzureCredentials.java:61:3:61:33 | new HardcodedAzureCredentials(...) [username] : String | HardcodedAzureCredentials.java:15:14:15:42 | parameter this [username] : String |
2828
| HardcodedAzureCredentials.java:63:3:63:33 | new HardcodedAzureCredentials(...) [clientSecret] : String | HardcodedAzureCredentials.java:43:14:43:38 | parameter this [clientSecret] : String |
29+
| HardcodedShiroKey.java:9:46:9:54 | "TEST123" : String | HardcodedShiroKey.java:9:46:9:65 | getBytes(...) |
30+
| HardcodedShiroKey.java:18:61:18:86 | "4AvVhmFLUs0KTA3Kprsdag==" : String | HardcodedShiroKey.java:18:46:18:87 | decode(...) |
31+
| HardcodedShiroKey.java:26:83:26:108 | "6ZmI6I2j5Y+R5aSn5ZOlAA==" : String | HardcodedShiroKey.java:26:46:26:109 | decode(...) |
2932
| Test.java:9:16:9:22 | "admin" : String | Test.java:12:13:12:15 | usr : String |
3033
| Test.java:9:16:9:22 | "admin" : String | Test.java:15:36:15:38 | usr |
3134
| Test.java:9:16:9:22 | "admin" : String | Test.java:17:39:17:41 | usr |
@@ -76,6 +79,12 @@ nodes
7679
| HardcodedAzureCredentials.java:61:3:61:33 | new HardcodedAzureCredentials(...) [clientSecret] : String | semmle.label | new HardcodedAzureCredentials(...) [clientSecret] : String |
7780
| HardcodedAzureCredentials.java:61:3:61:33 | new HardcodedAzureCredentials(...) [username] : String | semmle.label | new HardcodedAzureCredentials(...) [username] : String |
7881
| HardcodedAzureCredentials.java:63:3:63:33 | new HardcodedAzureCredentials(...) [clientSecret] : String | semmle.label | new HardcodedAzureCredentials(...) [clientSecret] : String |
82+
| HardcodedShiroKey.java:9:46:9:54 | "TEST123" : String | semmle.label | "TEST123" : String |
83+
| HardcodedShiroKey.java:9:46:9:65 | getBytes(...) | semmle.label | getBytes(...) |
84+
| HardcodedShiroKey.java:18:46:18:87 | decode(...) | semmle.label | decode(...) |
85+
| HardcodedShiroKey.java:18:61:18:86 | "4AvVhmFLUs0KTA3Kprsdag==" : String | semmle.label | "4AvVhmFLUs0KTA3Kprsdag==" : String |
86+
| HardcodedShiroKey.java:26:46:26:109 | decode(...) | semmle.label | decode(...) |
87+
| HardcodedShiroKey.java:26:83:26:108 | "6ZmI6I2j5Y+R5aSn5ZOlAA==" : String | semmle.label | "6ZmI6I2j5Y+R5aSn5ZOlAA==" : String |
7988
| Test.java:9:16:9:22 | "admin" : String | semmle.label | "admin" : String |
8089
| Test.java:10:17:10:24 | "123456" : String | semmle.label | "123456" : String |
8190
| Test.java:12:13:12:15 | usr : String | semmle.label | usr : String |
@@ -110,6 +119,9 @@ subpaths
110119
| HardcodedAzureCredentials.java:10:34:10:67 | "[email protected]" | HardcodedAzureCredentials.java:10:34:10:67 | "[email protected]" : String | HardcodedAzureCredentials.java:18:13:18:20 | username | Hard-coded value flows to $@. | HardcodedAzureCredentials.java:18:13:18:20 | username | sensitive API call |
111120
| HardcodedAzureCredentials.java:11:38:11:73 | "1n1.qAc~3Q-1t38aF79Xzv5AUEfR5-ct3_" | HardcodedAzureCredentials.java:11:38:11:73 | "1n1.qAc~3Q-1t38aF79Xzv5AUEfR5-ct3_" : String | HardcodedAzureCredentials.java:19:13:19:24 | clientSecret | Hard-coded value flows to $@. | HardcodedAzureCredentials.java:19:13:19:24 | clientSecret | sensitive API call |
112121
| HardcodedAzureCredentials.java:11:38:11:73 | "1n1.qAc~3Q-1t38aF79Xzv5AUEfR5-ct3_" | HardcodedAzureCredentials.java:11:38:11:73 | "1n1.qAc~3Q-1t38aF79Xzv5AUEfR5-ct3_" : String | HardcodedAzureCredentials.java:46:17:46:28 | clientSecret | Hard-coded value flows to $@. | HardcodedAzureCredentials.java:46:17:46:28 | clientSecret | sensitive API call |
122+
| HardcodedShiroKey.java:9:46:9:54 | "TEST123" | HardcodedShiroKey.java:9:46:9:54 | "TEST123" : String | HardcodedShiroKey.java:9:46:9:65 | getBytes(...) | Hard-coded value flows to $@. | HardcodedShiroKey.java:9:46:9:65 | getBytes(...) | sensitive API call |
123+
| HardcodedShiroKey.java:18:61:18:86 | "4AvVhmFLUs0KTA3Kprsdag==" | HardcodedShiroKey.java:18:61:18:86 | "4AvVhmFLUs0KTA3Kprsdag==" : String | HardcodedShiroKey.java:18:46:18:87 | decode(...) | Hard-coded value flows to $@. | HardcodedShiroKey.java:18:46:18:87 | decode(...) | sensitive API call |
124+
| HardcodedShiroKey.java:26:83:26:108 | "6ZmI6I2j5Y+R5aSn5ZOlAA==" | HardcodedShiroKey.java:26:83:26:108 | "6ZmI6I2j5Y+R5aSn5ZOlAA==" : String | HardcodedShiroKey.java:26:46:26:109 | decode(...) | Hard-coded value flows to $@. | HardcodedShiroKey.java:26:46:26:109 | decode(...) | sensitive API call |
113125
| Test.java:9:16:9:22 | "admin" | Test.java:9:16:9:22 | "admin" : String | Test.java:15:36:15:38 | usr | Hard-coded value flows to $@. | Test.java:15:36:15:38 | usr | sensitive API call |
114126
| Test.java:9:16:9:22 | "admin" | Test.java:9:16:9:22 | "admin" : String | Test.java:17:39:17:41 | usr | Hard-coded value flows to $@. | Test.java:17:39:17:41 | usr | sensitive API call |
115127
| Test.java:9:16:9:22 | "admin" | Test.java:9:16:9:22 | "admin" : String | Test.java:18:39:18:41 | usr | Hard-coded value flows to $@. | Test.java:18:39:18:41 | usr | sensitive API call |

‎java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedShiroKey.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import org.apache.shiro.web.mgt.CookieRememberMeManager;
2+
3+
4+
public class HardcodedShiroKey {
5+
6+
//BAD: hard-coded shiro key
7+
public void testHardcodedShiroKey(String input) {
8+
CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
9+
cookieRememberMeManager.setCipherKey("TEST123".getBytes());
10+
11+
}
12+
13+
14+
//BAD: hard-coded shiro key encoded by java.util.Base64
15+
public void testHardcodedbase64ShiroKey1(String input) {
16+
CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
17+
java.util.Base64.Decoder decoder = java.util.Base64.getDecoder();
18+
cookieRememberMeManager.setCipherKey(decoder.decode("4AvVhmFLUs0KTA3Kprsdag=="));
19+
20+
}
21+
22+
23+
//BAD: hard-coded shiro key encoded by org.apache.shiro.codec.Base64
24+
public void testHardcodedbase64ShiroKey2(String input) {
25+
CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
26+
cookieRememberMeManager.setCipherKey(org.apache.shiro.codec.Base64.decode("6ZmI6I2j5Y+R5aSn5ZOlAA=="));
27+
28+
}
29+
30+
//GOOD: random shiro key
31+
public void testRandomShiroKey(String input) {
32+
CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
33+
}
34+
35+
36+
37+
38+
39+
40+
}

‎java/ql/test/query-tests/security/CWE-798/semmle/tests/options

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
// semmle-extractor-options: --javac-args -cp ${testdir}/../../../../../stubs/amazon-aws-sdk-1.11.700:${testdir}/../../../../../stubs/azure-sdk-for-java
1+
// semmle-extractor-options: --javac-args -cp ${testdir}/../../../../../stubs/amazon-aws-sdk-1.11.700:${testdir}/../../../../../stubs/azure-sdk-for-java:${testdir}/../../../../../stubs/shiro-core-1.4.0

‎java/ql/test/stubs/shiro-core-1.4.0/org/apache/shiro/codec/Base64.java

Lines changed: 36 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎java/ql/test/stubs/shiro-core-1.4.0/org/apache/shiro/crypto/AbstractSymmetricCipherService.java

Lines changed: 28 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎java/ql/test/stubs/shiro-core-1.4.0/org/apache/shiro/crypto/AesCipherService.java

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)