Skip to content

Commit 0ca18b5

Browse files
author
John Bindel
committed
Issue#711 Appending char to StringBuilder or StringBuffer is not considered taint-free.
Character types can convey taint because libraries sometimes create String values after converting one String into characters.
1 parent 68628d3 commit 0ca18b5

File tree

3 files changed

+178
-8
lines changed

3 files changed

+178
-8
lines changed

‎findsecbugs-plugin/src/main/resources/taint-config/java-lang.txt‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
-- Following classes are immutable
2+
Ljava/lang/Character;:#IMMUTABLE
23
Ljava/lang/String;:#IMMUTABLE
34
Ljava/io/File;:#IMMUTABLE
45
Ljava/util/Locale;:#IMMUTABLE
@@ -10,7 +11,6 @@ Ljava/net/URL;:#IMMUTABLE
1011

1112
-- Following classes are SAFE (and also immutable in sense of being tainted)
1213
Ljava/lang/Boolean;:SAFE#IMMUTABLE
13-
Ljava/lang/Character;:SAFE#IMMUTABLE
1414
Ljava/lang/Double;:SAFE#IMMUTABLE
1515
Ljava/lang/Float;:SAFE#IMMUTABLE
1616
Ljava/lang/Integer;:SAFE#IMMUTABLE
@@ -82,7 +82,7 @@ java/lang/StringBuilder.append(Ljava/lang/StringBuffer;)Ljava/lang/StringBuilder
8282
java/lang/StringBuilder.append(Ljava/lang/CharSequence;)Ljava/lang/StringBuilder;:0,1#1
8383
java/lang/StringBuilder.append(Ljava/lang/CharSequence;II)Ljava/lang/StringBuilder;:2,3#3
8484
java/lang/StringBuilder.append(Z)Ljava/lang/StringBuilder;:1
85-
java/lang/StringBuilder.append(C)Ljava/lang/StringBuilder;:1
85+
java/lang/StringBuilder.append(C)Ljava/lang/StringBuilder;:0,1
8686
java/lang/StringBuilder.append(D)Ljava/lang/StringBuilder;:2
8787
java/lang/StringBuilder.append(F)Ljava/lang/StringBuilder;:1
8888
java/lang/StringBuilder.append(I)Ljava/lang/StringBuilder;:1
@@ -121,7 +121,7 @@ java/lang/StringBuffer.append(Ljava/lang/StringBuffer;)Ljava/lang/StringBuffer;:
121121
java/lang/StringBuffer.append(Ljava/lang/CharSequence;)Ljava/lang/StringBuffer;:0,1#1
122122
java/lang/StringBuffer.append(Ljava/lang/CharSequence;II)Ljava/lang/StringBuffer;:2,3#3
123123
java/lang/StringBuffer.append(Z)Ljava/lang/StringBuffer;:1
124-
java/lang/StringBuffer.append(C)Ljava/lang/StringBuffer;:1
124+
java/lang/StringBuffer.append(C)Ljava/lang/StringBuffer;:0,1
125125
java/lang/StringBuffer.append(D)Ljava/lang/StringBuffer;:2
126126
java/lang/StringBuffer.append(F)Ljava/lang/StringBuffer;:1
127127
java/lang/StringBuffer.append(I)Ljava/lang/StringBuffer;:1
@@ -177,7 +177,7 @@ java/lang/String.toUpperCase()Ljava/lang/String;:0
177177
java/lang/String.toUpperCase(Ljava/util/Locale;)Ljava/lang/String;:1
178178
java/lang/String.trim()Ljava/lang/String;:0
179179
java/lang/String.valueOf(Z)Ljava/lang/String;:SAFE
180-
java/lang/String.valueOf(C)Ljava/lang/String;:SAFE
180+
java/lang/String.valueOf(C)Ljava/lang/String;:0
181181
java/lang/String.valueOf(D)Ljava/lang/String;:SAFE
182182
java/lang/String.valueOf(F)Ljava/lang/String;:SAFE
183183
java/lang/String.valueOf(I)Ljava/lang/String;:SAFE
@@ -193,9 +193,9 @@ java/lang/CharSequence.subSequence(II)Ljava/lang/CharSequence;:2
193193

194194
java/lang/Boolean.toString()Ljava/lang/String;:SAFE
195195
java/lang/Boolean.toString(B)Ljava/lang/String;:SAFE
196-
java/lang/Character.getName(I)Ljava/lang/String;:SAFE
197-
java/lang/Character.toString()Ljava/lang/String;:SAFE
198-
java/lang/Character.toString(C)Ljava/lang/String;:SAFE
196+
java/lang/Character.getName(I)Ljava/lang/String;:0
197+
java/lang/Character.toString()Ljava/lang/String;:0
198+
java/lang/Character.toString(C)Ljava/lang/String;:0
199199
java/lang/Double.toString()Ljava/lang/String;:SAFE
200200
java/lang/Double.toString(D)Ljava/lang/String;:SAFE
201201
java/lang/Float.toHexString(F)Ljava/lang/String;:SAFE
@@ -258,4 +258,4 @@ java/util/ResourceBundle.getObject(Ljava/lang/String;)Ljava/lang/Object;:SAFE
258258
kotlin/text/StringsKt.replace$default(Ljava/lang/String;CCZILjava/lang/Object;)Ljava/lang/String;:5
259259
kotlin/text/StringsKt.replace$default(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZILjava/lang/Object;)Ljava/lang/String;:5
260260
kotlin/text/Regex.replace(Ljava/lang/CharSequence;Ljava/lang/String;)Ljava/lang/String;:1
261-
kotlin/text/Regex.<init>(Ljava/lang/String;)V:SAFE
261+
kotlin/text/Regex.<init>(Ljava/lang/String;)V:SAFE

‎findsecbugs-plugin/src/test/java/com/h3xstream/findsecbugs/taintanalysis/CharacterTaintPropagationTest.java‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/**
2+
* Find Security Bugs
3+
* Copyright (c) Philippe Arteau, All rights reserved.
4+
*
5+
* This library is free software; you can redistribute it and/or
6+
* modify it under the terms of the GNU Lesser General Public
7+
* License as published by the Free Software Foundation; either
8+
* version 3.0 of the License, or (at your option) any later version.
9+
*
10+
* This library is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
13+
* Lesser General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU Lesser General Public
16+
* License along with this library.
17+
*/
18+
package com.h3xstream.findsecbugs.taintanalysis;
19+
20+
import com.h3xstream.findbugs.test.BaseDetectorTest;
21+
import com.h3xstream.findbugs.test.EasyBugReporter;
22+
import org.testng.annotations.Test;
23+
24+
import static org.testng.Assert.assertTrue;
25+
26+
import static org.mockito.Mockito.*;
27+
28+
public class CharacterTaintPropagationTest extends BaseDetectorTest {
29+
30+
@Test
31+
public void validateCharacterTaintPropagation() throws Exception {
32+
//Locate test code
33+
String[] files = { getClassFilePath("testcode/taint/CharacterTaintPropagation") };
34+
35+
//Run the analysis
36+
EasyBugReporter reporter = spy(new SecurityReporter());
37+
38+
analyze(files, reporter);
39+
40+
verify(reporter, times(9)).doReportBug(
41+
bugDefinition().bugType("PATH_TRAVERSAL_IN").build());
42+
43+
verify(reporter, times(1)).doReportBug(
44+
bugDefinition().bugType("PATH_TRAVERSAL_IN")
45+
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromCharConcat")
46+
.withPriority("Medium")
47+
.build());
48+
49+
verify(reporter, times(1)).doReportBug(
50+
bugDefinition().bugType("PATH_TRAVERSAL_IN")
51+
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromChar")
52+
.withPriority("Medium")
53+
.build());
54+
55+
verify(reporter, times(1)).doReportBug(
56+
bugDefinition().bugType("PATH_TRAVERSAL_IN")
57+
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromCharToString")
58+
.withPriority("Medium")
59+
.build());
60+
61+
verify(reporter, times(1)).doReportBug(
62+
bugDefinition().bugType("PATH_TRAVERSAL_IN")
63+
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromCharGetName")
64+
.withPriority("Medium")
65+
.build());
66+
67+
verify(reporter, times(1)).doReportBug(
68+
bugDefinition().bugType("PATH_TRAVERSAL_IN")
69+
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromCharStringBuilder")
70+
.withPriority("Medium")
71+
.build());
72+
73+
verify(reporter, times(1)).doReportBug(
74+
bugDefinition().bugType("PATH_TRAVERSAL_IN")
75+
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromCharStringBuffer")
76+
.withPriority("Medium")
77+
.build());
78+
79+
verify(reporter, times(1)).doReportBug(
80+
bugDefinition().bugType("PATH_TRAVERSAL_IN")
81+
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromCharacterToString")
82+
.withPriority("Medium")
83+
.build());
84+
85+
verify(reporter, times(1)).doReportBug(
86+
bugDefinition().bugType("PATH_TRAVERSAL_IN")
87+
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromCharacterConcat")
88+
.withPriority("Medium")
89+
.build());
90+
91+
verify(reporter, times(1)).doReportBug(
92+
bugDefinition().bugType("PATH_TRAVERSAL_IN")
93+
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromString")
94+
.withPriority("Medium")
95+
.build());
96+
97+
verify(reporter, never()).doReportBug(
98+
bugDefinition().bugType("PATH_TRAVERSAL_IN")
99+
.inClass("CharacterTaintPropagation").inMethod("safeFileFromConstant")
100+
.build());
101+
102+
verify(reporter, never()).doReportBug(
103+
bugDefinition().bugType("PATH_TRAVERSAL_IN")
104+
.inClass("CharacterTaintPropagation").inMethod("safeFileFromConstantWithInt")
105+
.build());
106+
107+
}
108+
}

‎findsecbugs-samples-java/src/test/java/testcode/taint/CharacterTaintPropagation.java‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package testcode.taint;
2+
3+
import java.io.FileInputStream;
4+
import java.io.FileNotFoundException;
5+
6+
public class CharacterTaintPropagation {
7+
8+
private static final String SAFE_PART = "staticSafePart";
9+
10+
/** This should report PATH_TRAVERSAL_IN. */
11+
public FileInputStream unsafeFileFromCharConcat(final char unsafePart) throws FileNotFoundException {
12+
return new FileInputStream("safePart" + unsafePart);
13+
}
14+
15+
/** This should report PATH_TRAVERSAL_IN. */
16+
public FileInputStream unsafeFileFromChar(final char unsafePart) throws FileNotFoundException {
17+
return new FileInputStream(String.valueOf(unsafePart));
18+
}
19+
20+
/** This should report PATH_TRAVERSAL_IN. */
21+
public FileInputStream unsafeFileFromCharToString(final char unsafePart) throws FileNotFoundException {
22+
return new FileInputStream(Character.toString(unsafePart));
23+
}
24+
25+
/** This should report PATH_TRAVERSAL_IN. */
26+
public FileInputStream unsafeFileFromCharGetName(final int unsafePart) throws FileNotFoundException {
27+
return new FileInputStream(Character.getName(unsafePart));
28+
}
29+
30+
/** This should report PATH_TRAVERSAL_IN. */
31+
public FileInputStream unsafeFileFromCharStringBuilder(final char unsafePart) throws FileNotFoundException {
32+
return new FileInputStream(new StringBuilder("safePart").append(unsafePart).toString());
33+
}
34+
35+
/** This should report PATH_TRAVERSAL_IN. */
36+
public FileInputStream unsafeFileFromCharStringBuffer(final char unsafePart) throws FileNotFoundException {
37+
return new FileInputStream(new StringBuffer("safePart").append(unsafePart).toString());
38+
}
39+
40+
/** This should report PATH_TRAVERSAL_IN. */
41+
public FileInputStream unsafeFileFromCharacterToString(final Character unsafePart) throws FileNotFoundException {
42+
return new FileInputStream(unsafePart.toString());
43+
}
44+
45+
/** This should report PATH_TRAVERSAL_IN. */
46+
public FileInputStream unsafeFileFromCharacterConcat(final Character unsafePart) throws FileNotFoundException {
47+
return new FileInputStream("safePart" + unsafePart);
48+
}
49+
50+
/** This should report PATH_TRAVERSAL_IN. */
51+
public FileInputStream unsafeFileFromString(final String unsafePart) throws FileNotFoundException {
52+
return new FileInputStream("safePart" + unsafePart);
53+
}
54+
55+
public FileInputStream safeFileFromConstant() throws FileNotFoundException {
56+
return new FileInputStream(SAFE_PART);
57+
}
58+
59+
public FileInputStream safeFileFromConstantWithInt(final int safeInt) throws FileNotFoundException {
60+
return new FileInputStream(new StringBuilder(SAFE_PART).append(safeInt).toString());
61+
}
62+
}

0 commit comments

Comments
 (0)