Skip to content

Commit cda54aa

Browse files
atorralbaatorralba
committed
Add more tests
Also add missing close methods
1 parent 4e679a3 commit cda54aa

File tree

2 files changed

+210
-79
lines changed

2 files changed

+210
-79
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,13 @@ private predicate filesystemStore(DataFlow::Node file, Call store) {
7070
/** A method that closes a file. */
7171
private class CloseFileMethod extends Method {
7272
CloseFileMethod() {
73-
this.hasQualifiedName("java.io", ["RandomAccessFile", "FileOutputStream"], "close")
73+
this.hasQualifiedName("java.io", ["RandomAccessFile", "FileOutputStream", "PrintStream"],
74+
"close")
7475
or
7576
this.getDeclaringType().getASupertype*().hasQualifiedName("java.io", "Writer") and
7677
this.hasName("close")
7778
or
78-
this.hasQualifiedName("java.nio.file", "Files", "write")
79+
this.hasQualifiedName("java.nio.file", "Files", ["write", "writeString"])
7980
}
8081
}
8182

‎java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidFilesystemTest.java‎

Lines changed: 207 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@
33
import java.io.File;
44
import java.io.FileOutputStream;
55
import java.io.FileWriter;
6+
import java.io.OutputStream;
67
import java.io.OutputStreamWriter;
8+
import java.io.PrintStream;
79
import java.io.PrintWriter;
810
import java.io.RandomAccessFile;
911
import java.io.Writer;
12+
import java.nio.charset.Charset;
1013
import java.nio.charset.StandardCharsets;
1114
import java.nio.file.Files;
1215
import java.nio.file.Path;
@@ -21,122 +24,249 @@
2124

2225
public class CleartextStorageAndroidFilesystemTest extends Activity {
2326

24-
public void testWriteLocalFile1(String name, String password) throws Exception {
25-
// FileWriter
26-
FileWriter fw = new FileWriter("some_file.txt");
27-
fw.write(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
28-
fw.close();
29-
}
27+
public void testWriteLocalFile(Context context, String name, String password) throws Exception {
3028

31-
public void testWriteLocalFile2(String name, String password) throws Exception {
32-
// Nested writers
33-
Writer writer =
34-
new BufferedWriter(new OutputStreamWriter(new FileOutputStream("some_file.txt"), "utf-8"));
35-
writer.write(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
36-
writer.close();
37-
}
29+
// FileOutputStream
30+
{
31+
// java.io;FileOutputStream;false;FileOutputStream;;;Argument[0];create-file
32+
FileOutputStream os = new FileOutputStream("some_file.txt");
33+
// Nested writers
34+
Writer writer = new BufferedWriter(new OutputStreamWriter(os, "utf-8"));
35+
// java.io;FileOutputStream;false;write;;;Argument[0];write-file
36+
writer.write(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
37+
writer.close();
38+
}
3839

39-
public void testWriteLocalFile3(String name, String password) throws Exception {
40-
// try-with-resources
41-
try (FileWriter fw = new FileWriter("some_file.txt")) {
42-
fw.write(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
40+
// RandomAccessFile
41+
{
42+
// java.io;RandomAccessFile;false;RandomAccessFile;;;Argument[0];create-file
43+
RandomAccessFile f = new RandomAccessFile("some_file.txt", "r");
44+
String contents = name + ":" + password;
45+
// java.io;RandomAccessFile;false;write;;;Argument[0];write-file
46+
f.write(contents.getBytes()); // $ hasCleartextStorageAndroidFilesystem
47+
f.close();
48+
}
49+
{
50+
// try-with-resources
51+
try (RandomAccessFile f = new RandomAccessFile(new File("some_file.txt"), "r")) {
52+
// java.io;RandomAccessFile;false;writeBytes;;;Argument[0];write-file
53+
f.writeBytes(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
54+
}
55+
}
56+
{
57+
RandomAccessFile f = new RandomAccessFile("some_file.txt", "r");
58+
// java.io;RandomAccessFile;false;writeChars;;;Argument[0];write-file
59+
f.writeChars(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
60+
f.close();
61+
}
62+
{
63+
RandomAccessFile f = new RandomAccessFile("some_file.txt", "r");
64+
// java.io;RandomAccessFile;false;writeUTF;;;Argument[0];write-file
65+
f.writeUTF(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
66+
f.close();
67+
}
68+
69+
// FileWriter
70+
{
71+
// java.io;FileWriter;false;FileWriter;;;Argument[0];create-file
72+
FileWriter fw = new FileWriter("some_file.txt");
73+
// java.io;Writer;true;append;;;Argument[0];write-file
74+
fw.append(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
75+
fw.close();
76+
}
77+
{
78+
// try-with-resources
79+
try (FileWriter fw = new FileWriter("some_file.txt")) {
80+
// java.io;Writer;true;write;;;Argument[0];write-file
81+
fw.write(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
82+
}
83+
}
84+
85+
// PrintStream
86+
{
87+
// java.io;PrintStream;false;PrintStream;(File);;Argument[0];create-file"
88+
PrintStream ps = new PrintStream(new File("some_file.txt"));
89+
// java.io;PrintStream;true;append;;;Argument[0];write-file
90+
ps.append(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
91+
ps.close();
92+
}
93+
{
94+
// java.io;PrintStream;false;PrintStream;(File,String);;Argument[0];create-file
95+
PrintStream ps = new PrintStream(new File("some_file.txt"), "utf-8");
96+
// java.io;PrintStream;true;format;(String,Object[]);;Argument[0..1];write-file
97+
ps.format("%s:" + password, name); // $ hasCleartextStorageAndroidFilesystem
98+
ps.format("%s:%s", name, password); // $ hasCleartextStorageAndroidFilesystem
99+
ps.close();
100+
}
101+
{
102+
// java.io;PrintStream;false;PrintStream;(File,Charset);;Argument[0];create-file
103+
PrintStream ps = new PrintStream(new File("some_file.txt"), Charset.defaultCharset());
104+
// java.io;PrintStream;true;format;(Locale,String,Object[]);;Argument[1..2];write-file
105+
ps.format(Locale.US, "%s:" + password, name); // $ hasCleartextStorageAndroidFilesystem
106+
ps.format(Locale.US, "%s:%s", name, password); // $ hasCleartextStorageAndroidFilesystem
107+
ps.close();
108+
}
109+
{
110+
// java.io;PrintStream;false;PrintStream;(String);;Argument[0];create-file
111+
PrintStream ps = new PrintStream("some_file.txt");
112+
// java.io;PrintStream;true;print;;;Argument[0];write-file
113+
ps.print(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
114+
ps.close();
115+
}
116+
{
117+
// java.io;PrintStream;false;PrintStream;(String,String);;Argument[0];create-file
118+
PrintStream ps = new PrintStream("some_file.txt", "utf-8");
119+
// java.io;PrintStream;true;printf;(String,Object[]);;Argument[0..1];write-file
120+
ps.printf("%s:" + password, name); // $ hasCleartextStorageAndroidFilesystem
121+
ps.printf("%s:%s", name, password); // $ hasCleartextStorageAndroidFilesystem
122+
ps.close();
123+
}
124+
{
125+
// java.io;PrintStream;false;PrintStream;(String,Charset);;Argument[0];create-file
126+
PrintStream ps = new PrintStream("some_file.txt", Charset.defaultCharset());
127+
// java.io;PrintStream;true;printf;(Locale,String,Object[]);;Argument[1..2];write-file
128+
ps.printf(Locale.US, "%s:" + password, name); // $ hasCleartextStorageAndroidFilesystem
129+
ps.printf(Locale.US, "%s:%s", name, password); // $ hasCleartextStorageAndroidFilesystem
130+
ps.close();
131+
}
132+
{
133+
PrintStream ps = new PrintStream("some_file.txt");
134+
// java.io;PrintStream;true;println;;;Argument[0];write-file
135+
ps.println(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
136+
ps.close();
137+
}
138+
{
139+
PrintStream ps = new PrintStream("some_file.txt");
140+
String contents = name + ":" + password;
141+
// java.io;PrintStream;true;write;;;Argument[0];write-file
142+
ps.write(contents.getBytes()); // $ hasCleartextStorageAndroidFilesystem
143+
ps.close();
144+
}
145+
{
146+
PrintStream ps = new PrintStream("some_file.txt");
147+
String contents = name + ":" + password;
148+
// java.io;PrintStream;true;writeBytes;;;Argument[0];write-file
149+
ps.writeBytes(contents.getBytes()); // $ hasCleartextStorageAndroidFilesystem
150+
ps.close();
43151
}
44-
}
45152

46-
public void testWriteLocalFile4(String name, String password) throws Exception {
47153
// PrintWriter
48154
{
155+
// java.io;PrintWriter;false;PrintWriter;(File);;Argument[0];create-file
49156
PrintWriter pw = new PrintWriter(new File("some_file.txt"));
50-
pw.append(name);
51-
pw.append(":");
52-
pw.append(password); // $ hasCleartextStorageAndroidFilesystem
157+
// java.io;Writer;true;append;;;Argument[0];write-file
158+
pw.append(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
53159
pw.close();
54160
}
55161
{
162+
// try-with-resources
163+
try (PrintWriter pw = new PrintWriter(new File("some_file.txt"))) {
164+
// java.io;PrintWriter;false;format;(String,Object[]);;Argument[0..1];write-file
165+
pw.format("%s:" + password, name); // $ hasCleartextStorageAndroidFilesystem
166+
pw.format("%s:%s", name, password); // $ hasCleartextStorageAndroidFilesystem
167+
}
168+
}
169+
{
170+
// java.io;PrintWriter;false;PrintWriter;(File,String);;Argument[0];create-file
56171
PrintWriter pw = new PrintWriter(new File("some_file.txt"), "utf-8");
57-
pw.print(name);
58-
pw.print(":");
59-
pw.print(password); // $ hasCleartextStorageAndroidFilesystem
172+
// java.io;PrintWriter;false;format;(Locale,String,Object[]);;Argument[1..2];write-file
173+
pw.format(Locale.US, "%s:" + password, name); // $ hasCleartextStorageAndroidFilesystem
174+
pw.format(Locale.US, "%s:%s", name, password); // $ hasCleartextStorageAndroidFilesystem
175+
pw.close();
176+
}
177+
{
178+
// java.io;PrintWriter;false;PrintWriter;(File,Charset);;Argument[0];create-file
179+
PrintWriter pw = new PrintWriter(new File("some_file.txt"), Charset.defaultCharset());
180+
// java.io;PrintWriter;false;print;;;Argument[0];write-file
181+
pw.print(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
60182
pw.close();
61183
}
184+
62185
{
186+
// java.io;PrintWriter;false;PrintWriter;(String);;Argument[0];create-file
63187
PrintWriter pw = new PrintWriter("some_file.txt");
64-
pw.println(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
188+
// java.io;PrintWriter;false;printf;(String,Object[]);;Argument[0..1];write-file
189+
pw.printf("%s:" + password, name); // $ hasCleartextStorageAndroidFilesystem
190+
pw.printf("%s:%s", name, password); // $ hasCleartextStorageAndroidFilesystem
65191
pw.close();
66192
}
67193
{
194+
// java.io;PrintWriter;false;PrintWriter;(String,String);;Argument[0];create-file
68195
PrintWriter pw = new PrintWriter("some_file.txt", "utf-8");
69-
pw.write(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
196+
// java.io;PrintWriter;false;printf;(Locale,String,Object[]);;Argument[1..2];write-file
197+
pw.printf(Locale.US, "%s:" + password, name); // $ hasCleartextStorageAndroidFilesystem
198+
pw.printf(Locale.US, "%s:%s", name, password); // $ hasCleartextStorageAndroidFilesystem
70199
pw.close();
71200
}
72201
{
73-
PrintWriter pw = new PrintWriter("some_file.txt");
74-
pw.format(Locale.US, "%s:%s", name, password); // $ hasCleartextStorageAndroidFilesystem
202+
// java.io;PrintWriter;false;PrintWriter;(String,Charset);;Argument[0];create-file
203+
PrintWriter pw = new PrintWriter("some_file.txt", Charset.defaultCharset());
204+
// java.io;PrintWriter;false;println;;;Argument[0];write-file
205+
pw.println(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
75206
pw.close();
76207
}
77208
{
78-
try (PrintWriter pw = new PrintWriter("some_file.txt")) {
79-
pw.format("%s:%s", name, password); // $ hasCleartextStorageAndroidFilesystem
80-
}
209+
PrintWriter pw = new PrintWriter("some_file.txt");
210+
// java.io;Writer;true;write;;;Argument[0];write-file
211+
pw.write(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
212+
pw.close();
81213
}
82-
}
83214

84-
public void testWriteLocalFile5(String name, String password) throws Exception {
85215
// java.nio.files.Files
86-
String contents = name + ":" + password;
87-
// @formatter:off
88216
{
89-
Files.write(Path.of("some_file.txt"), contents.getBytes()); // $ hasCleartextStorageAndroidFilesystem
217+
// java.nio.file;Files;false;newBufferedWriter;;;Argument[0];create-file
218+
BufferedWriter bw = Files.newBufferedWriter(Path.of("some_file.txt"));
219+
bw.write(name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
220+
bw.close();
90221
}
91222
{
92-
Files.write(Path.of("some_file.txt"), List.of(contents)); // $ hasCleartextStorageAndroidFilesystem
223+
// java.nio.file;Files;false;newOutputStream;;;Argument[0];create-file
224+
// try-with-resources
225+
try (OutputStream os = Files.newOutputStream(Path.of("some_file.txt"))) {
226+
String contents = name + ":" + password;
227+
os.write(contents.getBytes());
228+
}
93229
}
94-
// @formatter:on
95-
}
96-
97-
public void testWriteLocalFile6(String name, String password) throws Exception {
98-
// RandomAccessFile
99-
String contents = name + ":" + password;
100230
{
101-
RandomAccessFile f = new RandomAccessFile("some_file.txt", "r");
102-
f.write(contents.getBytes()); // $ hasCleartextStorageAndroidFilesystem
103-
f.close();
231+
Path path = Path.of("some_file.txt");
232+
String contents = name + ":" + password;
233+
// java.nio.file;Files;false;write;;;Argument[0];create-file
234+
// java.nio.file;Files;false;write;;;Argument[1];write-file",
235+
Files.write(path, contents.getBytes()); // $ hasCleartextStorageAndroidFilesystem
104236
}
105237
{
106-
// @formatter:off
107-
try (RandomAccessFile f = new RandomAccessFile(new File("some_file.txt"), "r")) {
108-
f.write(contents.getBytes(), 0, contents.length()); // $ hasCleartextStorageAndroidFilesystem
109-
}
110-
// @formatter:on
238+
Path path = Path.of("some_file.txt");
239+
String contents = name + ":" + password;
240+
Files.write(path, List.of(contents)); // $ hasCleartextStorageAndroidFilesystem
241+
}
242+
{
243+
Path path = Path.of("some_file.txt");
244+
// java.nio.file;Files;false;writeString;;;Argument[0];create-file
245+
// java.nio.file;Files;false;writeString;;;Argument[1];write-file"
246+
Files.writeString(path, name + ":" + password); // $ hasCleartextStorageAndroidFilesystem
111247
}
112-
}
113-
114-
public void testWriteLocalFileSafe1(String name, String password) throws Exception {
115-
// Using hashing
116-
FileWriter fw = new FileWriter("some_file.txt");
117-
fw.write(name + ":" + hash(password)); // Safe
118-
fw.close();
119-
}
120-
121-
public void testWriteLocalFileSafe2(String name, String password) throws Exception {
122-
// Nested writers
123-
Writer writer =
124-
new BufferedWriter(new OutputStreamWriter(new ByteArrayOutputStream(), "utf-8"));
125-
writer.write(name + ":" + password); // Safe - not writing to a file
126-
writer.close();
127-
}
128248

129-
public void testWriteLocalFileSafe3(Context context, String name, String password)
130-
throws Exception {
131-
File file = new File("some_file.txt");
132-
String contents = name + ":" + password;
133-
EncryptedFile encryptedFile = new EncryptedFile.Builder(file, context, "some_key",
134-
FileEncryptionScheme.AES256_GCM_HKDF_4KB).build();
135-
FileOutputStream encryptedOutputStream = encryptedFile.openFileOutput();
136-
encryptedOutputStream.write(contents.getBytes()); // Safe
249+
// Safe writes
250+
{
251+
FileWriter fw = new FileWriter("some_file.txt");
252+
fw.write(name + ":" + hash(password)); // Safe - using a hash
253+
fw.close();
254+
}
255+
{
256+
Writer writer = new OutputStreamWriter(new ByteArrayOutputStream(), "utf-8");
257+
writer.write(name + ":" + password); // Safe - not writing to a file
258+
writer.close();
259+
}
260+
{
261+
File file = new File("some_file.txt");
262+
String contents = name + ":" + password;
263+
EncryptedFile encryptedFile = new EncryptedFile.Builder(file, context, "some_key",
264+
FileEncryptionScheme.AES256_GCM_HKDF_4KB).build();
265+
FileOutputStream encryptedOutputStream = encryptedFile.openFileOutput();
266+
encryptedOutputStream.write(contents.getBytes()); // Safe - using EncryptedFile
267+
}
137268
}
138269

139-
140270
private static String hash(String cleartext) throws Exception {
141271
MessageDigest digest = MessageDigest.getInstance("SHA-256");
142272
byte[] hash = digest.digest(cleartext.getBytes(StandardCharsets.UTF_8));

0 commit comments

Comments
 (0)