Skip to content

Commit 168fc41

Browse files
x-f1v3smowton
x-f1v3
authored and
smowton
committed
Apply suggestions from code review
1 parent f3bde56 commit 168fc41

File tree

7 files changed

+80
-35
lines changed

7 files changed

+80
-35
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`)
3+
This query finds hard-coded cipher key for shiro. This vulnerability can lead to arbitrary code execution.

‎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: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import java
1414
import semmle.code.java.dataflow.DataFlow
1515
import HardcodedCredentials
1616
import DataFlow::PathGraph
17+
import semmle.code.java.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
18+
1719

1820
class HardcodedCredentialApiCallConfiguration extends DataFlow::Configuration {
1921
HardcodedCredentialApiCallConfiguration() { this = "HardcodedCredentialApiCallConfiguration" }
@@ -27,10 +29,10 @@ class HardcodedCredentialApiCallConfiguration extends DataFlow::Configuration {
2729

2830
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
2931
node1.asExpr().getType() instanceof TypeString and
30-
exists(MethodAccess ma | ma.getMethod().hasName(["getBytes", "toCharArray","decode"]) |
32+
(exists(MethodAccess ma | ma.getMethod().hasName(["getBytes", "toCharArray"]) |
3133
node2.asExpr() = ma and
32-
(ma.getQualifier() = node1.asExpr() or ma.getAnArgument() = node1.asExpr())
33-
)
34+
ma.getQualifier() = node1.asExpr()) or FlowSummaryImpl::Private::Steps::summaryThroughStep(node1, node2, false))
35+
3436
}
3537

3638
override predicate isBarrier(DataFlow::Node n) {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -490,8 +490,7 @@ private predicate javaApiCallableCryptoKeyParam(string s) {
490490
s = "sun.security.provider.JavaKeyStore;engineSetKeyEntry(String, byte[], Certificate[]);1" or
491491
s = "sun.security.tools.keytool.Main;recoverKey(String, char[], char[]);2" or
492492
s = "sun.security.tools.keytool.Main;getKeyPasswd(String, String, char[]);2" or
493-
s = "sun.security.x509.X509Key;decode(byte[]);0" or
494-
s = "org.apache.shiro.mgt.AbstractRememberMeManager;setCipherKey(byte[]);0"
493+
s = "sun.security.x509.X509Key;decode(byte[]);0"
495494
}
496495

497496
/**
@@ -514,5 +513,6 @@ private predicate otherApiCallableCredentialParam(string s) {
514513
s = "com.amazonaws.auth.BasicAWSCredentials;BasicAWSCredentials(String, String);1" or
515514
s = "com.azure.identity.UsernamePasswordCredentialBuilder;username(String);0" or
516515
s = "com.azure.identity.UsernamePasswordCredentialBuilder;password(String);0" or
517-
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"
518518
}

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +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:8:46:8:54 | "TEST123" : String | HardcodedShiroKey.java:8:46:8:65 | getBytes(...) |
30-
| HardcodedShiroKey.java:16:60:16:85 | "4AvVhmFLUs0KTA3Kprsdag==" : String | HardcodedShiroKey.java:16:46:16:86 | decode(...) |
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(...) |
3132
| Test.java:9:16:9:22 | "admin" : String | Test.java:12:13:12:15 | usr : String |
3233
| Test.java:9:16:9:22 | "admin" : String | Test.java:15:36:15:38 | usr |
3334
| Test.java:9:16:9:22 | "admin" : String | Test.java:17:39:17:41 | usr |
@@ -78,10 +79,12 @@ nodes
7879
| HardcodedAzureCredentials.java:61:3:61:33 | new HardcodedAzureCredentials(...) [clientSecret] : String | semmle.label | new HardcodedAzureCredentials(...) [clientSecret] : String |
7980
| HardcodedAzureCredentials.java:61:3:61:33 | new HardcodedAzureCredentials(...) [username] : String | semmle.label | new HardcodedAzureCredentials(...) [username] : String |
8081
| HardcodedAzureCredentials.java:63:3:63:33 | new HardcodedAzureCredentials(...) [clientSecret] : String | semmle.label | new HardcodedAzureCredentials(...) [clientSecret] : String |
81-
| HardcodedShiroKey.java:8:46:8:54 | "TEST123" : String | semmle.label | "TEST123" : String |
82-
| HardcodedShiroKey.java:8:46:8:65 | getBytes(...) | semmle.label | getBytes(...) |
83-
| HardcodedShiroKey.java:16:46:16:86 | decode(...) | semmle.label | decode(...) |
84-
| HardcodedShiroKey.java:16:60:16:85 | "4AvVhmFLUs0KTA3Kprsdag==" : String | semmle.label | "4AvVhmFLUs0KTA3Kprsdag==" : 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 |
8588
| Test.java:9:16:9:22 | "admin" : String | semmle.label | "admin" : String |
8689
| Test.java:10:17:10:24 | "123456" : String | semmle.label | "123456" : String |
8790
| Test.java:12:13:12:15 | usr : String | semmle.label | usr : String |
@@ -116,8 +119,9 @@ subpaths
116119
| 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 |
117120
| 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 |
118121
| 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 |
119-
| HardcodedShiroKey.java:8:46:8:54 | "TEST123" | HardcodedShiroKey.java:8:46:8:54 | "TEST123" : String | HardcodedShiroKey.java:8:46:8:65 | getBytes(...) | Hard-coded value flows to $@. | HardcodedShiroKey.java:8:46:8:65 | getBytes(...) | sensitive API call |
120-
| HardcodedShiroKey.java:16:60:16:85 | "4AvVhmFLUs0KTA3Kprsdag==" | HardcodedShiroKey.java:16:60:16:85 | "4AvVhmFLUs0KTA3Kprsdag==" : String | HardcodedShiroKey.java:16:46:16:86 | decode(...) | Hard-coded value flows to $@. | HardcodedShiroKey.java:16:46:16:86 | decode(...) | 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 |
121125
| 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 |
122126
| 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 |
123127
| 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: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,38 @@
11
import org.apache.shiro.web.mgt.CookieRememberMeManager;
22

3+
34
public class HardcodedShiroKey {
45

56
//BAD: hard-coded shiro key
6-
public void testHardcodedShiroKey(String input) {
7-
CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
7+
public void testHardcodedShiroKey(String input) {
8+
CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
89
cookieRememberMeManager.setCipherKey("TEST123".getBytes());
910

10-
}
11-
12-
13-
//BAD: hard-coded shiro key
14-
public void testHardcodedbase64ShiroKey(String input) {
15-
CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
16-
cookieRememberMeManager.setCipherKey(Base64.decode("4AvVhmFLUs0KTA3Kprsdag=="));
17-
18-
}
11+
}
1912

20-
//GOOD: random shiro key
21-
public void testRandomShiroKey(String input) {
22-
CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
23-
}
2413

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=="));
2519

20+
}
2621

27-
static class Base64 {
2822

29-
static byte[] decode(String str){
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=="));
3027

31-
byte[] x = new byte[1024];
28+
}
3229

33-
return x;
30+
//GOOD: random shiro key
31+
public void testRandomShiroKey(String input) {
32+
CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
33+
}
3434

35-
}
3635

37-
}
3836

3937

4038

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

0 commit comments

Comments
 (0)