hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 19:26:02 +02:00
Code Issues Packages Projects Releases Wiki Activity

Apply suggestions from code review

Browse Source
This commit is contained in:
f1v3
2021-05-24 17:02:06 +08:00
committed by Chris Smowton
parent f3bde56de9
commit 168fc4170d
7 changed files with 81 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
java/change-notes/2021-05-24-hardcoded-shiro-key-in-api-call.md Normal file
View File

@@ -0,0 +1,3 @@
lgtm,codescanning
* The query "Hard-coded credential in API call" (`java/hardcoded-credential-api-call`)
This query finds hard-coded cipher key for shiro. This vulnerability can lead to arbitrary code execution.

2
java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll
View File

@@ -309,6 +309,8 @@ private predicate summaryModelCsv(string row) {
"java.util;Base64$Decoder;false;decode;(ByteBuffer);;Argument[0];ReturnValue;taint",
"java.util;Base64$Decoder;false;decode;(String);;Argument[0];ReturnValue;taint",
"java.util;Base64$Decoder;false;wrap;(InputStream);;Argument[0];ReturnValue;taint",
"cn.hutool.core.codec;Base64;true;decode;;;Argument[0];ReturnValue;taint",
"org.apache.shiro.codec;Base64;false;decode;(String);;Argument[0];ReturnValue;taint",
"org.apache.commons.codec;Encoder;true;encode;(Object);;Argument[0];ReturnValue;taint",
"org.apache.commons.codec;Decoder;true;decode;(Object);;Argument[0];ReturnValue;taint",
"org.apache.commons.codec;BinaryEncoder;true;encode;(byte[]);;Argument[0];ReturnValue;taint",

8
java/ql/src/Security/CWE/CWE-798/HardcodedCredentialsApiCall.ql
View File

@@ -14,6 +14,8 @@ import java
import semmle.code.java.dataflow.DataFlow
import HardcodedCredentials
import DataFlow::PathGraph
import semmle.code.java.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
class HardcodedCredentialApiCallConfiguration extends DataFlow::Configuration {
HardcodedCredentialApiCallConfiguration() { this = "HardcodedCredentialApiCallConfiguration" }
@@ -27,10 +29,10 @@ class HardcodedCredentialApiCallConfiguration extends DataFlow::Configuration {
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
node1.asExpr().getType() instanceof TypeString and
exists(MethodAccess ma | ma.getMethod().hasName(["getBytes", "toCharArray","decode"]) |
(exists(MethodAccess ma | ma.getMethod().hasName(["getBytes", "toCharArray"]) |
node2.asExpr() = ma and
(ma.getQualifier() = node1.asExpr() or ma.getAnArgument() = node1.asExpr())
)
ma.getQualifier() = node1.asExpr()) or FlowSummaryImpl::Private::Steps::summaryThroughStep(node1, node2, false))
}
override predicate isBarrier(DataFlow::Node n) {

6
java/ql/src/Security/CWE/CWE-798/SensitiveApi.qll
View File

@@ -490,8 +490,7 @@ private predicate javaApiCallableCryptoKeyParam(string s) {
s = "sun.security.provider.JavaKeyStore;engineSetKeyEntry(String, byte[], Certificate[]);1" or
s = "sun.security.tools.keytool.Main;recoverKey(String, char[], char[]);2" or
s = "sun.security.tools.keytool.Main;getKeyPasswd(String, String, char[]);2" or
s = "sun.security.x509.X509Key;decode(byte[]);0" or
s = "org.apache.shiro.mgt.AbstractRememberMeManager;setCipherKey(byte[]);0"
s = "sun.security.x509.X509Key;decode(byte[]);0"
}
/**
@@ -514,5 +513,6 @@ private predicate otherApiCallableCredentialParam(string s) {
s = "com.amazonaws.auth.BasicAWSCredentials;BasicAWSCredentials(String, String);1" or
s = "com.azure.identity.UsernamePasswordCredentialBuilder;username(String);0" or
s = "com.azure.identity.UsernamePasswordCredentialBuilder;password(String);0" or
s = "com.azure.identity.ClientSecretCredentialBuilder;clientSecret(String);0"
s = "com.azure.identity.ClientSecretCredentialBuilder;clientSecret(String);0" or
s = "org.apache.shiro.mgt.AbstractRememberMeManager;setCipherKey(byte[]);0"
}

20
java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCredentialsApiCall.expected
View File

@@ -26,8 +26,9 @@ edges
| HardcodedAzureCredentials.java:61:3:61:33 | new HardcodedAzureCredentials(...) [clientSecret] : String | HardcodedAzureCredentials.java:15:14:15:42 | parameter this [clientSecret] : String |
| HardcodedAzureCredentials.java:61:3:61:33 | new HardcodedAzureCredentials(...) [username] : String | HardcodedAzureCredentials.java:15:14:15:42 | parameter this [username] : String |
| HardcodedAzureCredentials.java:63:3:63:33 | new HardcodedAzureCredentials(...) [clientSecret] : String | HardcodedAzureCredentials.java:43:14:43:38 | parameter this [clientSecret] : String |
| HardcodedShiroKey.java:8:46:8:54 | "TEST123" : String | HardcodedShiroKey.java:8:46:8:65 | getBytes(...) |
| HardcodedShiroKey.java:16:60:16:85 | "4AvVhmFLUs0KTA3Kprsdag==" : String | HardcodedShiroKey.java:16:46:16:86 | decode(...) |
| HardcodedShiroKey.java:9:46:9:54 | "TEST123" : String | HardcodedShiroKey.java:9:46:9:65 | getBytes(...) |
| HardcodedShiroKey.java:18:61:18:86 | "4AvVhmFLUs0KTA3Kprsdag==" : String | HardcodedShiroKey.java:18:46:18:87 | decode(...) |
| HardcodedShiroKey.java:26:83:26:108 | "6ZmI6I2j5Y+R5aSn5ZOlAA==" : String | HardcodedShiroKey.java:26:46:26:109 | decode(...) |
| Test.java:9:16:9:22 | "admin" : String | Test.java:12:13:12:15 | usr : String |
| Test.java:9:16:9:22 | "admin" : String | Test.java:15:36:15:38 | usr |
| Test.java:9:16:9:22 | "admin" : String | Test.java:17:39:17:41 | usr |
@@ -78,10 +79,12 @@ nodes
| HardcodedAzureCredentials.java:61:3:61:33 | new HardcodedAzureCredentials(...) [clientSecret] : String | semmle.label | new HardcodedAzureCredentials(...) [clientSecret] : String |
| HardcodedAzureCredentials.java:61:3:61:33 | new HardcodedAzureCredentials(...) [username] : String | semmle.label | new HardcodedAzureCredentials(...) [username] : String |
| HardcodedAzureCredentials.java:63:3:63:33 | new HardcodedAzureCredentials(...) [clientSecret] : String | semmle.label | new HardcodedAzureCredentials(...) [clientSecret] : String |
| HardcodedShiroKey.java:8:46:8:54 | "TEST123" : String | semmle.label | "TEST123" : String |
| HardcodedShiroKey.java:8:46:8:65 | getBytes(...) | semmle.label | getBytes(...) |
| HardcodedShiroKey.java:16:46:16:86 | decode(...) | semmle.label | decode(...) |
| HardcodedShiroKey.java:16:60:16:85 | "4AvVhmFLUs0KTA3Kprsdag==" : String | semmle.label | "4AvVhmFLUs0KTA3Kprsdag==" : String |
| HardcodedShiroKey.java:9:46:9:54 | "TEST123" : String | semmle.label | "TEST123" : String |
| HardcodedShiroKey.java:9:46:9:65 | getBytes(...) | semmle.label | getBytes(...) |
| HardcodedShiroKey.java:18:46:18:87 | decode(...) | semmle.label | decode(...) |
| HardcodedShiroKey.java:18:61:18:86 | "4AvVhmFLUs0KTA3Kprsdag==" : String | semmle.label | "4AvVhmFLUs0KTA3Kprsdag==" : String |
| HardcodedShiroKey.java:26:46:26:109 | decode(...) | semmle.label | decode(...) |
| HardcodedShiroKey.java:26:83:26:108 | "6ZmI6I2j5Y+R5aSn5ZOlAA==" : String | semmle.label | "6ZmI6I2j5Y+R5aSn5ZOlAA==" : String |
| Test.java:9:16:9:22 | "admin" : String | semmle.label | "admin" : String |
| Test.java:10:17:10:24 | "123456" : String | semmle.label | "123456" : String |
| Test.java:12:13:12:15 | usr : String | semmle.label | usr : String |
@@ -116,8 +119,9 @@ subpaths
| HardcodedAzureCredentials.java:10:34:10:67 | "username@example.onmicrosoft.com" | HardcodedAzureCredentials.java:10:34:10:67 | "username@example.onmicrosoft.com" : String | HardcodedAzureCredentials.java:18:13:18:20 | username | Hard-coded value flows to $@. | HardcodedAzureCredentials.java:18:13:18:20 | username | sensitive API call |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |

42
java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedShiroKey.java
View File

@@ -1,40 +1,38 @@
import org.apache.shiro.web.mgt.CookieRememberMeManager;
public class HardcodedShiroKey {
//BAD: hard-coded shiro key
public void testHardcodedShiroKey(String input) {
CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
public void testHardcodedShiroKey(String input) {
CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
cookieRememberMeManager.setCipherKey("TEST123".getBytes());
}
//BAD: hard-coded shiro key
public void testHardcodedbase64ShiroKey(String input) {
CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
cookieRememberMeManager.setCipherKey(Base64.decode("4AvVhmFLUs0KTA3Kprsdag=="));
}
//GOOD: random shiro key
public void testRandomShiroKey(String input) {
CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
}
}
//BAD: hard-coded shiro key encoded by java.util.Base64
public void testHardcodedbase64ShiroKey1(String input) {
CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
java.util.Base64.Decoder decoder = java.util.Base64.getDecoder();
cookieRememberMeManager.setCipherKey(decoder.decode("4AvVhmFLUs0KTA3Kprsdag=="));
static class Base64 {
}
static byte[] decode(String str){
byte[] x = new byte[1024];
//BAD: hard-coded shiro key encoded by org.apache.shiro.codec.Base64
public void testHardcodedbase64ShiroKey2(String input) {
CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
cookieRememberMeManager.setCipherKey(org.apache.shiro.codec.Base64.decode("6ZmI6I2j5Y+R5aSn5ZOlAA=="));
return x;
}
//GOOD: random shiro key
public void testRandomShiroKey(String input) {
CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
}
}
}

36
java/ql/test/stubs/shiro-core-1.4.0/org/apache/shiro/codec/Base64.java generated Normal file
View File

@@ -0,0 +1,36 @@
//
// Source code recreated from a .class file by IntelliJ IDEA
// (powered by Fernflower decompiler)
//
package org.apache.shiro.codec;
public class Base64 {
static final int CHUNK_SIZE = 76;
static final byte[] CHUNK_SEPARATOR = "\r\n".getBytes();
private static final int BASELENGTH = 255;
private static final int LOOKUPLENGTH = 64;
private static final int EIGHTBIT = 8;
private static final int SIXTEENBIT = 16;
private static final int TWENTYFOURBITGROUP = 24;
private static final int FOURBYTE = 4;
private static final int SIGN = -128;
private static final byte PAD = 61;
private static final byte[] base64Alphabet = new byte[255];
private static final byte[] lookUpBase64Alphabet = new byte[64];
public Base64() {
}
public static byte[] decode(String base64Encoded) {
byte[] bytes = new byte[1024];
return decode(bytes);
}
public static byte[] decode(byte[] base64Data) {
return base64Data;
}
}