Skip to content

Commit d6c5132

Browse files
joefarebrotherjoefarebrother
authored
Merge pull request #10684 from joefarebrother/android-keyboard-cache
Java: Add query for Sensitive Keyboard Cache
2 parents dfc72ed + cc96037 commit d6c5132

File tree

13 files changed

+296
-0
lines changed

13 files changed

+296
-0
lines changed

‎java/ql/lib/semmle/code/java/security/SensitiveKeyboardCacheQuery.qll‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
/** Definitions for the keyboard cache query */
2+
3+
import java
4+
import semmle.code.java.dataflow.DataFlow
5+
import semmle.code.java.security.SensitiveActions
6+
import semmle.code.xml.AndroidManifest
7+
8+
/** An Android Layout XML file. */
9+
private class AndroidLayoutXmlFile extends XmlFile {
10+
AndroidLayoutXmlFile() { this.getRelativePath().matches("%/res/layout/%.xml") }
11+
}
12+
13+
/** A component declared in an Android layout file. */
14+
class AndroidLayoutXmlElement extends XmlElement {
15+
AndroidXmlAttribute id;
16+
17+
AndroidLayoutXmlElement() {
18+
this.getFile() instanceof AndroidLayoutXmlFile and
19+
id = this.getAttribute("id")
20+
}
21+
22+
/** Gets the ID of this component. */
23+
string getId() { result = id.getValue() }
24+
25+
/** Gets the class of this component. */
26+
Class getClass() {
27+
this.getName() = "view" and
28+
this.getAttribute("class").getValue() = result.getQualifiedName()
29+
or
30+
this.getName() = result.getQualifiedName()
31+
or
32+
result.hasQualifiedName(["android.widget", "android.view"], this.getName())
33+
}
34+
}
35+
36+
/** An XML element that represents an editable text field. */
37+
class AndroidEditableXmlElement extends AndroidLayoutXmlElement {
38+
AndroidEditableXmlElement() {
39+
this.getClass().getASourceSupertype*().hasQualifiedName("android.widget", "EditText")
40+
}
41+
42+
/** Gets the input type of this field, if any. */
43+
string getInputType() { result = this.getAttribute("inputType").(AndroidXmlAttribute).getValue() }
44+
}
45+
46+
/** A `findViewById` or `requireViewById` method on `Activity` or `View`. */
47+
private class FindViewMethod extends Method {
48+
FindViewMethod() {
49+
this.hasQualifiedName("android.view", "View", ["findViewById", "requireViewById"])
50+
or
51+
exists(Method m |
52+
m.hasQualifiedName("android.app", "Activity", ["findViewById", "requireViewById"]) and
53+
this = m.getAnOverride*()
54+
)
55+
}
56+
}
57+
58+
/** Gets a use of the view that has the given id. */
59+
private MethodAccess getAUseOfViewWithId(string id) {
60+
exists(string name, NestedClass r_id, Field id_field |
61+
id = "@+id/" + name and
62+
result.getMethod() instanceof FindViewMethod and
63+
r_id.getEnclosingType().hasName("R") and
64+
r_id.hasName("id") and
65+
id_field.getDeclaringType() = r_id and
66+
id_field.hasName(name)
67+
|
68+
DataFlow::localExprFlow(id_field.getAnAccess(), result.getArgument(0))
69+
)
70+
}
71+
72+
/** Gets the argument of a use of `setInputType` called on the view with the given id. */
73+
private Argument setInputTypeForId(string id) {
74+
exists(MethodAccess setInputType |
75+
setInputType.getMethod().hasQualifiedName("android.widget", "TextView", "setInputType") and
76+
DataFlow::localExprFlow(getAUseOfViewWithId(id), setInputType.getQualifier()) and
77+
result = setInputType.getArgument(0)
78+
)
79+
}
80+
81+
/** Holds if the given field is a constant flag indicating that an input with this type will not be cached. */
82+
private predicate inputTypeFieldNotCached(Field f) {
83+
f.getDeclaringType().hasQualifiedName("android.text", "InputType") and
84+
(
85+
not f.getName().matches("%TEXT%")
86+
or
87+
f.getName().matches("%PASSWORD%")
88+
or
89+
f.hasName("TYPE_TEXT_FLAG_NO_SUGGESTIONS")
90+
)
91+
}
92+
93+
/** Configuration that finds uses of `setInputType` for non cached fields. */
94+
private class GoodInputTypeConf extends DataFlow::Configuration {
95+
GoodInputTypeConf() { this = "GoodInputTypeConf" }
96+
97+
override predicate isSource(DataFlow::Node node) {
98+
inputTypeFieldNotCached(node.asExpr().(FieldAccess).getField())
99+
}
100+
101+
override predicate isSink(DataFlow::Node node) { node.asExpr() = setInputTypeForId(_) }
102+
103+
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
104+
exists(OrBitwiseExpr bitOr |
105+
node1.asExpr() = bitOr.getAChildExpr() and
106+
node2.asExpr() = bitOr
107+
)
108+
}
109+
}
110+
111+
/** Gets a regex indicating that an input field may contain sensitive data. */
112+
private string getInputSensitiveInfoRegex() {
113+
result =
114+
[
115+
getCommonSensitiveInfoRegex(),
116+
"(?i).*(bank|credit|debit|(pass(wd|word|code|phrase))|security).*"
117+
]
118+
}
119+
120+
/** Holds if input using the given input type (as written in XML) is not stored in the keyboard cache. */
121+
bindingset[ty]
122+
private predicate inputTypeNotCached(string ty) {
123+
not ty.matches("%text%")
124+
or
125+
ty.regexpMatch("(?i).*(nosuggestions|password).*")
126+
}
127+
128+
/** Gets an input field whose contents may be sensitive and may be stored in the keyboard cache. */
129+
AndroidEditableXmlElement getASensitiveCachedInput() {
130+
result.getId().regexpMatch(getInputSensitiveInfoRegex()) and
131+
(
132+
not inputTypeNotCached(result.getInputType()) and
133+
not exists(GoodInputTypeConf conf, DataFlow::Node src, DataFlow::Node sink |
134+
conf.hasFlow(src, sink) and
135+
sink.asExpr() = setInputTypeForId(result.getId())
136+
)
137+
)
138+
}

‎java/ql/src/Security/CWE/CWE-524/Example.xml‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<LinearLayout
3+
xmlns:android="http://schemas.android.com/apk/res/android"
4+
xmlns:app="http://schemas.android.com/apk/res-auto">
5+
6+
<!-- BAD: This password field uses the `text` input type, which allows the input to be saved to the keyboard cache. -->
7+
<EditText
8+
android:id="@+id/password_bad"
9+
android:inputType="text"/>
10+
11+
<!-- GOOD: This password field uses the `textPassword` input type, which ensures that the input is not saved to the keyboard cache. -->
12+
<EditText
13+
android:id="@+id/password_good"
14+
android:inputType="textPassword"/>
15+
</LinearLayout>

‎java/ql/src/Security/CWE/CWE-524/SensitiveKeyboardCache.qhelp‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>When a user enters information in a text input field on an Android application, their input is saved to a keyboard cache which provides autocomplete suggestions and predictions. There is a risk that sensitive user data, such as passwords or banking information, may be leaked to other applications via the keyboard cache.</p>
8+
9+
</overview>
10+
<recommendation>
11+
12+
<p>For input fields expected to accept sensitive information, use input types such as <code>"textNoSuggestions"</code> (or <code>"textPassword"</code> for a password) to ensure the input does not get stored in the keyboard cache.</p>
13+
<p>Optionally, instead of declaring an input type through XML, you can set the input type in your code using <code>TextView.setInputType()</code>.</p>
14+
</recommendation>
15+
<example>
16+
17+
<p>In the following example, the field labeled BAD allows the password to be saved to the keyboard cache,
18+
whereas the field labeled GOOD uses the <code>"textPassword"</code> input type to ensure the password is not cached.</p>
19+
20+
<sample src="Example.xml" />
21+
22+
</example>
23+
<references>
24+
25+
<li>
26+
OWASP Mobile Application Security Testing Guide: <a href="https://github.com/OWASP/owasp-mastg/blob/b7a93a2e5e0557cc9a12e55fc3f6675f6986bb86/Document/0x05d-Testing-Data-Storage.md#determining-whether-the-keyboard-cache-is-disabled-for-text-input-fields-mstg-storage-5">Determining Whether the Keyboard Cache Is Disabled for Text Input Fields</a>.
27+
</li>
28+
<li>
29+
Android Developers: <a href="https://developer.android.com/reference/android/widget/TextView#attr_android:inputType">android:inputType attribute documentation.</a>
30+
</li>
31+
32+
</references>
33+
</qhelp>

‎java/ql/src/Security/CWE/CWE-524/SensitiveKeyboardCache.ql‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/**
2+
* @name Android sensitive keyboard cache
3+
* @description Allowing the keyboard to cache sensitive information may result in information leaks to other applications.
4+
* @kind problem
5+
* @problem.severity warning
6+
* @security-severity 8.1
7+
* @id java/android/sensitive-keyboard-cache
8+
* @tags security
9+
* external/cwe/cwe-524
10+
* @precision medium
11+
*/
12+
13+
import java
14+
import semmle.code.java.security.SensitiveKeyboardCacheQuery
15+
16+
from AndroidEditableXmlElement el
17+
where el = getASensitiveCachedInput()
18+
select el, "This input field may contain sensitive information that is saved to the keyboard cache."

‎java/ql/src/change-notes/2022-10-07-sensitive-keyboard-cache.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added a new query, `java/android/sensitive-keyboard-cache`, to detect instances of sensitive information possibly being saved to the keyboard cache.

‎java/ql/test/query-tests/security/CWE-524/AndroidManifest.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
3+
xmlns:tools="http://schemas.android.com/tools"
4+
package="com.example.test">
5+
</manifest>

‎java/ql/test/query-tests/security/CWE-524/R.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package com.example.test;
2+
3+
public final class R {
4+
public static final class id {
5+
public static final int test7_password = 1;
6+
public static final int test8_password = 2;
7+
}
8+
}

‎java/ql/test/query-tests/security/CWE-524/SensitiveKeyboardCache.expected‎

Whitespace-only changes.

‎java/ql/test/query-tests/security/CWE-524/SensitiveKeyboardCache.ql‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import java
2+
import semmle.code.java.security.SensitiveKeyboardCacheQuery
3+
import TestUtilities.InlineExpectationsTest
4+
5+
class SensitiveKeyboardCacheTest extends InlineExpectationsTest {
6+
SensitiveKeyboardCacheTest() { this = "SensitiveKeyboardCacheTest" }
7+
8+
override string getARelevantTag() { result = "hasResult" }
9+
10+
override predicate hasActualResult(Location loc, string element, string tag, string value) {
11+
exists(AndroidEditableXmlElement el |
12+
el = getASensitiveCachedInput() and
13+
loc = el.getLocation() and
14+
element = el.toString() and
15+
tag = "hasResult" and
16+
value = ""
17+
)
18+
}
19+
}

‎java/ql/test/query-tests/security/CWE-524/Test.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package com.example.test;
2+
import android.app.Activity;
3+
import android.os.Bundle;
4+
import android.widget.EditText;
5+
import android.view.View;
6+
import android.text.InputType;
7+
8+
class Test extends Activity {
9+
public void onCreate(Bundle b) {
10+
EditText test7pw = findViewById(R.id.test7_password);
11+
test7pw.setInputType(InputType.TYPE_CLASS_TEXT | InputType.TYPE_TEXT_FLAG_NO_SUGGESTIONS);
12+
13+
EditText test8pw = requireViewById(R.id.test8_password);
14+
test8pw.setInputType(InputType.TYPE_CLASS_TEXT | InputType.TYPE_TEXT_VARIATION_PASSWORD);
15+
}
16+
}

0 commit comments

Comments
 (0)