diff --git a/java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.ql b/java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.ql index 0e606984a4d..3fd10b5d45b 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-927/SensitiveBroadcast.ql @@ -8,16 +8,10 @@ */ import java -import semmle.code.java.frameworks.android.Intent +import semmle.code.java.dataflow.DataFlow3 import semmle.code.java.dataflow.TaintTracking - -/** - * Gets a regular expression for matching common names of variables that indicate the value being held contains sensitive information. - */ -private string getCommonSensitiveInfoRegex() { - result = "(?i).*challenge|pass(wd|word|code|phrase)(?!.*question).*" or - result = "(?i).*(token|username|userid|secret).*" -} +import semmle.code.java.frameworks.android.Intent +import semmle.code.java.security.SensitiveActions /** * Gets regular expression for matching names of Android variables that indicate the value being held contains sensitive information. @@ -48,16 +42,13 @@ class PutBundleExtraMethodAccess extends MethodAccess { class SensitiveInfoExpr extends Expr { SensitiveInfoExpr() { exists(Variable v | this = v.getAnAccess() | - ( - v.getName().toLowerCase().regexpMatch(getCommonSensitiveInfoRegex()) or - v.getName().toLowerCase().regexpMatch(getAndroidSensitiveInfoRegex()) - ) + v.getName().regexpMatch([getCommonSensitiveInfoRegex(), getAndroidSensitiveInfoRegex()]) ) } } /** - * The method access of `context.sendBroadcast`. + * A method access of the `context.sendBroadcast` family. */ class SendBroadcastMethodAccess extends MethodAccess { SendBroadcastMethodAccess() { @@ -71,17 +62,21 @@ private class NullArgFlowConfig extends DataFlow2::Configuration { override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof NullLiteral } - override predicate isSink(DataFlow::Node sink) { any() } + override predicate isSink(DataFlow::Node sink) { + exists(SendBroadcastMethodAccess ma | sink.asExpr() = ma.getAnArgument()) + } } -private class EmptyArrayArgFlowConfig extends DataFlow2::Configuration { +private class EmptyArrayArgFlowConfig extends DataFlow3::Configuration { EmptyArrayArgFlowConfig() { this = "Flow configuration with an empty array argument" } override predicate isSource(DataFlow::Node src) { src.asExpr().(ArrayCreationExpr).getFirstDimensionSize() = 0 } - override predicate isSink(DataFlow::Node sink) { any() } + override predicate isSink(DataFlow::Node sink) { + exists(SendBroadcastMethodAccess ma | sink.asExpr() = ma.getAnArgument()) + } } /** @@ -110,7 +105,7 @@ predicate isSensitiveBroadcastSink(DataFlow::Node sink) { config.hasFlow(_, DataFlow::exprNode(ma.getArgument(1))) // sendBroadcastWithMultiplePermissions(Intent intent, String[] receiverPermissions) ) or - //Method calls of `sendOrderedBroadcast` whose second argument is always `receiverPermission` + // Method calls of `sendOrderedBroadcast` whose second argument is always `receiverPermission` ma.getMethod().hasName("sendOrderedBroadcast") and ( // sendOrderedBroadcast(Intent intent, String receiverPermission) or sendOrderedBroadcast(Intent intent, String receiverPermission, BroadcastReceiver resultReceiver, Handler scheduler, int initialCode, String initialData, Bundle initialExtras) @@ -123,7 +118,7 @@ predicate isSensitiveBroadcastSink(DataFlow::Node sink) { ma.getNumArgument() = 8 ) or - //Method call of `sendOrderedBroadcastAsUser(Intent intent, UserHandle user, String receiverPermission, BroadcastReceiver resultReceiver, Handler scheduler, int initialCode, String initialData, Bundle initialExtras)` + // Method call of `sendOrderedBroadcastAsUser(Intent intent, UserHandle user, String receiverPermission, BroadcastReceiver resultReceiver, Handler scheduler, int initialCode, String initialData, Bundle initialExtras)` ma.getMethod().hasName("sendOrderedBroadcastAsUser") and exists(NullArgFlowConfig conf | conf.hasFlow(_, DataFlow::exprNode(ma.getArgument(2)))) ) diff --git a/java/ql/src/semmle/code/java/security/SensitiveActions.qll b/java/ql/src/semmle/code/java/security/SensitiveActions.qll index 90380439500..946799672b7 100644 --- a/java/ql/src/semmle/code/java/security/SensitiveActions.qll +++ b/java/ql/src/semmle/code/java/security/SensitiveActions.qll @@ -27,6 +27,14 @@ private string nonSuspicious() { result = "%crypt%" } +/** + * Gets a regular expression for matching common names of variables that indicate the value being held contains sensitive information. + */ +string getCommonSensitiveInfoRegex() { + result = "(?i).*challenge|pass(wd|word|code|phrase)(?!.*question).*" or + result = "(?i).*(token|username|userid|secret).*" +} + /** An expression that might contain sensitive data. */ abstract class SensitiveExpr extends Expr { } diff --git a/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.expected b/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.expected index 346faa17f42..867d03494ed 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.expected +++ b/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.expected @@ -1,12 +1,12 @@ | SensitiveBroadcast.java:13:31:13:36 | intent | SensitiveBroadcast.java:11:34:11:38 | token : String | SensitiveBroadcast.java:13:31:13:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:11:34:11:38 | token | sensitive information | | SensitiveBroadcast.java:13:31:13:36 | intent | SensitiveBroadcast.java:12:41:12:52 | refreshToken : String | SensitiveBroadcast.java:13:31:13:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:12:41:12:52 | refreshToken | sensitive information | -| SensitiveBroadcast.java:25:31:25:36 | intent | SensitiveBroadcast.java:23:33:23:40 | username : String | SensitiveBroadcast.java:25:31:25:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:23:33:23:40 | username | sensitive information | +| SensitiveBroadcast.java:25:31:25:36 | intent | SensitiveBroadcast.java:23:33:23:40 | userName : String | SensitiveBroadcast.java:25:31:25:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:23:33:23:40 | userName | sensitive information | | SensitiveBroadcast.java:25:31:25:36 | intent | SensitiveBroadcast.java:24:32:24:39 | password : String | SensitiveBroadcast.java:25:31:25:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:24:32:24:39 | password | sensitive information | -| SensitiveBroadcast.java:39:31:39:36 | intent | SensitiveBroadcast.java:36:40:36:47 | username : String | SensitiveBroadcast.java:39:31:39:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:36:40:36:47 | username | sensitive information | -| SensitiveBroadcast.java:39:31:39:36 | intent | SensitiveBroadcast.java:37:39:37:46 | password : String | SensitiveBroadcast.java:39:31:39:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:37:39:37:46 | password | sensitive information | -| SensitiveBroadcast.java:89:54:89:59 | intent | SensitiveBroadcast.java:87:33:87:40 | username : String | SensitiveBroadcast.java:89:54:89:59 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:87:33:87:40 | username | sensitive information | -| SensitiveBroadcast.java:89:54:89:59 | intent | SensitiveBroadcast.java:88:32:88:39 | password : String | SensitiveBroadcast.java:89:54:89:59 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:88:32:88:39 | password | sensitive information | -| SensitiveBroadcast.java:102:54:102:59 | intent | SensitiveBroadcast.java:99:33:99:40 | username : String | SensitiveBroadcast.java:102:54:102:59 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:99:33:99:40 | username | sensitive information | -| SensitiveBroadcast.java:102:54:102:59 | intent | SensitiveBroadcast.java:100:32:100:39 | password : String | SensitiveBroadcast.java:102:54:102:59 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:100:32:100:39 | password | sensitive information | -| SensitiveBroadcast.java:116:54:116:59 | intent | SensitiveBroadcast.java:112:33:112:40 | username : String | SensitiveBroadcast.java:116:54:116:59 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:112:33:112:40 | username | sensitive information | -| SensitiveBroadcast.java:116:54:116:59 | intent | SensitiveBroadcast.java:113:32:113:39 | password : String | SensitiveBroadcast.java:116:54:116:59 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:113:32:113:39 | password | sensitive information | +| SensitiveBroadcast.java:37:31:37:36 | intent | SensitiveBroadcast.java:35:41:35:45 | email : String | SensitiveBroadcast.java:37:31:37:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:35:41:35:45 | email | sensitive information | +| SensitiveBroadcast.java:49:31:49:36 | intent | SensitiveBroadcast.java:47:33:47:40 | username : String | SensitiveBroadcast.java:49:31:49:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:47:33:47:40 | username | sensitive information | +| SensitiveBroadcast.java:49:31:49:36 | intent | SensitiveBroadcast.java:48:32:48:39 | password : String | SensitiveBroadcast.java:49:31:49:36 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:48:32:48:39 | password | sensitive information | +| SensitiveBroadcast.java:95:54:95:59 | intent | SensitiveBroadcast.java:94:35:94:40 | ticket : String | SensitiveBroadcast.java:95:54:95:59 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:94:35:94:40 | ticket | sensitive information | +| SensitiveBroadcast.java:108:54:108:59 | intent | SensitiveBroadcast.java:105:33:105:40 | username : String | SensitiveBroadcast.java:108:54:108:59 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:105:33:105:40 | username | sensitive information | +| SensitiveBroadcast.java:108:54:108:59 | intent | SensitiveBroadcast.java:106:32:106:39 | password : String | SensitiveBroadcast.java:108:54:108:59 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:106:32:106:39 | password | sensitive information | +| SensitiveBroadcast.java:139:54:139:59 | intent | SensitiveBroadcast.java:134:40:134:47 | username : String | SensitiveBroadcast.java:139:54:139:59 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:134:40:134:47 | username | sensitive information | +| SensitiveBroadcast.java:139:54:139:59 | intent | SensitiveBroadcast.java:135:39:135:46 | password : String | SensitiveBroadcast.java:139:54:139:59 | intent | Sending $@ to broadcast. | SensitiveBroadcast.java:135:39:135:46 | password | sensitive information | diff --git a/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.java b/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.java index 12cec1de601..907fa927a49 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.java +++ b/java/ql/test/experimental/query-tests/security/CWE-927/SensitiveBroadcast.java @@ -4,7 +4,7 @@ import android.os.Bundle; class SensitiveBroadcast { - //Tests broadcast of access token with intent extra. + // BAD - Tests broadcast of access token with intent extra. public void sendBroadcast1(Context context, String token, String refreshToken) { Intent intent = new Intent(); intent.setAction("com.example.custom_action"); @@ -13,34 +13,44 @@ class SensitiveBroadcast { context.sendBroadcast(intent); } - //Tests broadcast of sensitive user information with intent extra. + // BAD - Tests broadcast of sensitive user information with intent extra. public void sendBroadcast2(Context context) { - String username = "test123"; + String userName = "test123"; String password = "abc12345"; Intent intent = new Intent(); intent.setAction("com.example.custom_action"); - intent.putExtra("name", username); + intent.putExtra("name", userName); intent.putExtra("pwd", password); context.sendBroadcast(intent); } - //Tests broadcast of sensitive user information with extra bundle. + // BAD - Tests broadcast of email information with extra bundle. public void sendBroadcast3(Context context) { + String email = "user123@example.com"; + + Intent intent = new Intent(); + intent.setAction("com.example.custom_action"); + Bundle bundle = new Bundle(); + bundle.putCharSequence("email", email); + intent.putExtras(bundle); + context.sendBroadcast(intent); + } + + // BAD - Tests broadcast of sensitive user information with null permission. + public void sendBroadcast4(Context context) { String username = "test123"; String password = "abc12345"; Intent intent = new Intent(); intent.setAction("com.example.custom_action"); - Bundle bundle = new Bundle(); - bundle.putCharSequence("name", username); - bundle.putCharSequence("pwd", password); - intent.putExtras(bundle); - context.sendBroadcast(intent); - } + intent.putExtra("name", username); + intent.putExtra("pwd", password); + context.sendBroadcast(intent, null); + } - //Tests broadcast of sensitive user information with permission using string literal. - public void sendBroadcast4(Context context) { + // GOOD - Tests broadcast of sensitive user information with permission using string literal. + public void sendBroadcast5(Context context) { String username = "test123"; String password = "abc12345"; @@ -51,21 +61,19 @@ class SensitiveBroadcast { context.sendBroadcast(intent, "com.example.user_permission"); } - //Tests broadcast of sensitive user information with permission using string object. - public void sendBroadcast5(Context context) { - String username = "test123"; - String password = "abc12345"; + // GOOD - Tests broadcast of access ticket with permission using string object. + public void sendBroadcast6(Context context) { + String ticket = "Tk9UIFNlY3VyZSBUaWNrZXQ="; Intent intent = new Intent(); intent.setAction("com.example.custom_action"); - intent.putExtra("name", username); - intent.putExtra("pwd", password); + intent.putExtra("ticket", ticket); String perm = "com.example.user_permission"; context.sendBroadcast(intent, perm); } - //Tests broadcast of sensitive user information to a specific application. - public void sendBroadcast6(Context context) { + // GOOD - Tests broadcast of sensitive user information to a specific application. + public void sendBroadcast7(Context context) { String username = "test123"; String password = "abc12345"; @@ -77,20 +85,18 @@ class SensitiveBroadcast { context.sendBroadcast(intent); } - //Tests broadcast of sensitive user information with multiple permissions using direct empty array initialization. - public void sendBroadcast7(Context context) { - String username = "test123"; - String password = "abc12345"; + // BAD - Tests broadcast of access ticket with multiple permissions using direct empty array initialization. + public void sendBroadcast8(Context context) { + String ticket = "Tk9UIFNlY3VyZSBUaWNrZXQ="; Intent intent = new Intent(); intent.setAction("com.example.custom_action"); - intent.putExtra("name", username); - intent.putExtra("pwd", password); + intent.putExtra("ticket", ticket); context.sendBroadcastWithMultiplePermissions(intent, new String[]{}); } - //Tests broadcast of sensitive user information with multiple permissions using empty array initialization through a variable. - public void sendBroadcast8(Context context) { + // BAD - Tests broadcast of sensitive user information with multiple permissions using empty array initialization through a variable. + public void sendBroadcast9(Context context) { String username = "test123"; String password = "abc12345"; @@ -102,22 +108,39 @@ class SensitiveBroadcast { context.sendBroadcastWithMultiplePermissions(intent, perms); } - //Tests broadcast of sensitive user information with multiple permissions using empty array initialization through two variables. - public void sendBroadcast9(Context context) { + // GOOD - Tests broadcast of sensitive user information with multiple permissions. + public void sendBroadcast10(Context context) { String username = "test123"; String password = "abc12345"; Intent intent = new Intent(); intent.setAction("com.example.custom_action"); - intent.putExtra("name", username); - intent.putExtra("pwd", password); + Bundle bundle = new Bundle(); + bundle.putCharSequence("name", username); + bundle.putCharSequence("pwd", password); + intent.putExtras(bundle); + String[] perms = new String[]{"com.example.custom_action", "com.example.custom_action2"}; + context.sendBroadcastWithMultiplePermissions(intent, perms); + } + + // BAD - Tests broadcast of sensitive user information with multiple permissions using empty array initialization through two variables. + public void sendBroadcast11(Context context) { + String username = "test123"; + String password = "abc12345"; + + Intent intent = new Intent(); + intent.setAction("com.example.custom_action"); + Bundle bundle = new Bundle(); + bundle.putCharSequence("name", username); + bundle.putCharSequence("pwd", password); + intent.putExtras(bundle); String[] perms = new String[0]; String[] perms2 = perms; context.sendBroadcastWithMultiplePermissions(intent, perms2); } - //Tests broadcast of sensitive user information with ordered broadcast. - public void sendBroadcast10(Context context) { + // GOOD - Tests broadcast of sensitive user information with ordered broadcast. + public void sendBroadcast12(Context context) { String username = "test123"; String password = "abc12345";