Java: Add query for exposure of sensitive information to android notifications #15281
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,48 @@ extensions: | |
| - ["android.app", "FragmentTransaction", True, "replace", "(int,Class,Bundle,String)", "", "Argument[1]", "fragment-injection", "manual"] | ||
| - ["android.app", "FragmentTransaction", True, "replace", "(int,Fragment)", "", "Argument[1]", "fragment-injection", "manual"] | ||
| - ["android.app", "FragmentTransaction", True, "replace", "(int,Fragment,String)", "", "Argument[1]", "fragment-injection", "manual"] | ||
| - ["android.app", "Notification$Action", True, "Action", "(int,CharSequence,PendingIntent)", "", "Argument[1]", "notification", "manual"] | ||
| - ["android.app", "Notification$Action$Builder", True, "Builder", "(Icon,CharSequence,PendingIntent)", "", "Argument[1]", "notification", "manual"] | ||
| - ["android.app", "Notification$Action$Builder", True, "Builder", "(int,CharSequence,PendingIntent)", "", "Argument[1]", "notification", "manual"] | ||
| - ["android.app", "Notification$Action$Builder", True, "addExtras", "(Bundle)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$BigPictureStyle", True, "setBigContentTitle", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$BigPictureStyle", True, "setContentDescription", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$BigPictureStyle", True, "setSummaryText", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$BigTextStyle", True, "bigText", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$BigTextStyle", True, "setBigContentTitle", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$BigTextStyle", True, "setSummaryText", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "addAction", "(int,CharSequence,PendingIntent)", "", "Argument[1]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "addExtras", "(Bundle)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "setCategory", "(String)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "setChannelId", "(String)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "setContent", "(RemoteViews)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "setContentInfo", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "setContentText", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "setContentTitle", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "setCustomBigContentView", "(RemoteViews)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "setCustomContentView", "(RemoteViews)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "setCustomHeadsUpContentView", "(RemoteViews)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "setExtras", "(Bundle)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "setGroup", "(String)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "setRemoteInputHistory", "(CharSequence[])", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "setSettingsText", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "setSortKey", "(String)", "", "Argument[0]", "notification", "manual"] | ||
|
There was a problem hiding this comment. Can the sort key be recovered from the notification? And even if so, this doesn't very likely to receive sensitive data, does it? There was a problem hiding this comment. It can with getSortKey. Though it does indeed seem unlikely to receive sensitive data. There was a problem hiding this comment. Well, the model won't hurt, you never know what people can use as sort key 😄 |
||
| - ["android.app", "Notification$Builder", True, "setSubText", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "setTicker", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$Builder", True, "setTicker", "(CharSequence,RemoteViews)", "", "Argument[0..1]", "notification", "manual"] | ||
| - ["android.app", "Notification$CallStyle", True, "setVerificationText", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$InboxStyle", True, "addLine", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$InboxStyle", True, "setBigContentTitle", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$InboxStyle", True, "setSummaryText", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$MediaStyle", True, "setRemotePlaybackInfo", "(CharSequence,int,PendingIntent)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$MessagingStyle", True, "MessagingStyle", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$MessagingStyle", True, "addMessage", "(CharSequence,long,CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$MessagingStyle", True, "addMessage", "(CharSequence,long,CharSequence)", "", "Argument[2]", "notification", "manual"] | ||
| - ["android.app", "Notification$MessagingStyle", True, "addMessage", "(CharSequence,long,Person)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$MessagingStyle", True, "setConversationTitle", "(CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$MessagingStyle$Message", True, "Message", "(CharSequence,long,CharSequence)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "Notification$MessagingStyle$Message", True, "Message", "(CharSequence,long,CharSequence)", "", "Argument[2]", "notification", "manual"] | ||
| - ["android.app", "Notification$MessagingStyle$Message", True, "Message", "(CharSequence,long,Person)", "", "Argument[0]", "notification", "manual"] | ||
| - ["android.app", "NotificationManager", True, "notify", "(String,int,Notification)", "", "Argument[2]", "pending-intents", "manual"] | ||
| - ["android.app", "NotificationManager", True, "notify", "(int,Notification)", "", "Argument[1]", "pending-intents", "manual"] | ||
| - ["android.app", "NotificationManager", True, "notifyAsPackage", "(String,String,int,Notification)", "", "Argument[3]", "pending-intents", "manual"] | ||
|
|
@@ -38,6 +80,7 @@ extensions: | |
| - ["android.app", "PendingIntent", False, "send", "(Context,int,Intent,PendingIntent$OnFinished,Handler)", "", "Argument[2]", "pending-intents", "manual"] | ||
| - ["android.app", "PendingIntent", False, "send", "(Context,int,Intent,PendingIntent$OnFinished,Handler,String)", "", "Argument[2]", "pending-intents", "manual"] | ||
| - ["android.app", "PendingIntent", False, "send", "(Context,int,Intent,PendingIntent$OnFinished,Handler,String,Bundle)", "", "Argument[2]", "pending-intents", "manual"] | ||
|
|
||
| - addsTo: | ||
| pack: codeql/java-all | ||
| extensible: summaryModel | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| /** Definitions for Android Sensitive UI queries */ | ||
|
There was a problem hiding this comment. I assume you plan to add more configurations here and that's why this library file has a more general name. There was a problem hiding this comment. Indeed; #15396 uses the same file |
||
|
|
||
| import java | ||
| private import semmle.code.java.dataflow.ExternalFlow | ||
| private import semmle.code.java.dataflow.TaintTracking | ||
| private import semmle.code.java.security.SensitiveActions | ||
|
|
||
| /** A configuration for tracking sensitive information to system notifications. */ | ||
| private module NotificationTrackingConfig implements DataFlow::ConfigSig { | ||
| predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SensitiveExpr } | ||
|
|
||
| predicate isSink(DataFlow::Node sink) { sinkNode(sink, "notification") } | ||
|
|
||
joefarebrother marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) { | ||
| isSink(node) and exists(c) | ||
| } | ||
|
|
||
| predicate isBarrierIn(DataFlow::Node node) { isSource(node) } | ||
| } | ||
|
|
||
| /** Taint tracking flow for sensitive data flowing to system notifications. */ | ||
| module NotificationTracking = TaintTracking::Global<NotificationTrackingConfig>; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| // BAD: `password` is exposed in a notification. | ||
| void confirmPassword(String password) { | ||
| NotificationManager manager = NotificationManager.from(this); | ||
| manager.send( | ||
| new Notification.Builder(this, CHANNEL_ID) | ||
| .setContentText("Your password is: " + password) | ||
| .build()); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| <!DOCTYPE qhelp PUBLIC | ||
| "-//Semmle//qhelp//EN" | ||
| "qhelp.dtd"> | ||
| <qhelp> | ||
| <overview> | ||
| <p> | ||
| Sensitive information such as passwords or two-factor authentication (2FA) codes should not be exposed in a system notification. | ||
| Notifications should not be considered secure, as other untrusted applications may be able to use a | ||
| <code>NotificationListenerService</code> to read the contents of notifications. | ||
| </p> | ||
| </overview> | ||
|
|
||
| <recommendation> | ||
| <p> | ||
| Do not expose sensitive data in notifications. | ||
| </p> | ||
| </recommendation> | ||
|
|
||
| <example> | ||
| <p> | ||
| In the following sample, the <code>password</code> is sent as part of a notification. | ||
| This can allow another application to read this password. | ||
| </p> | ||
|
|
||
| <sample src="AndroidSensitiveNotifications.java"/> | ||
| </example> | ||
|
|
||
| <references> | ||
| <li> | ||
| OWASP Mobile Application Security: <a href="https://mas.owasp.org/MASTG/Android/0x05d-Testing-Data-Storage/#app-notifications">Android Data Storage - Application Notifications</a> | ||
| </li> | ||
| </references> | ||
|
|
||
| </qhelp> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| /** | ||
| * @name Exposure of sensitive information to notifications | ||
| * @id java/android/sensitive-notification | ||
| * @kind path-problem | ||
| * @description Sensitive information exposed in a system notification can be read by an unauthorized application. | ||
| * @problem.severity error | ||
| * @precision medium | ||
| * @security-severity 6.5 | ||
| * @tags security | ||
| * external/cwe/cwe-200 | ||
| */ | ||
|
|
||
| import java | ||
| import java | ||
| import semmle.code.java.security.SensitiveUiQuery | ||
| import NotificationTracking::PathGraph | ||
|
|
||
| from NotificationTracking::PathNode source, NotificationTracking::PathNode sink | ||
| where NotificationTracking::flowPath(source, sink) | ||
| select sink, source, sink, "This $@ is exposed in a system notification.", source, | ||
| "sensitive information" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| --- | ||
| category: newQuery | ||
| --- | ||
| * Added a new query `java/android/sensitive-notification` to detect instances of sensitive data being exposed through Android notifications. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can a
RemoteViewscontain sensitive data?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think existing models could track it, but it seems like it would theoretically be able to.