Skip to content

Commit

Permalink
Issue#711 Appending char to StringBuilder or StringBuffer is not cons…
Browse files Browse the repository at this point in the history
…idered taint-free.

Character types can convey taint because libraries sometimes create String values after converting one String into characters.
  • Loading branch information
John Bindel committed Aug 18, 2023
1 parent 68628d3 commit 0ca18b5
Show file tree
Hide file tree
Showing 3 changed files with 178 additions and 8 deletions.
16 changes: 8 additions & 8 deletions findsecbugs-plugin/src/main/resources/taint-config/java-lang.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
-- Following classes are immutable
Ljava/lang/Character;:#IMMUTABLE
Ljava/lang/String;:#IMMUTABLE
Ljava/io/File;:#IMMUTABLE
Ljava/util/Locale;:#IMMUTABLE
Expand All @@ -10,7 +11,6 @@ Ljava/net/URL;:#IMMUTABLE

-- Following classes are SAFE (and also immutable in sense of being tainted)
Ljava/lang/Boolean;:SAFE#IMMUTABLE
Ljava/lang/Character;:SAFE#IMMUTABLE
Ljava/lang/Double;:SAFE#IMMUTABLE
Ljava/lang/Float;:SAFE#IMMUTABLE
Ljava/lang/Integer;:SAFE#IMMUTABLE
Expand Down Expand Up @@ -82,7 +82,7 @@ java/lang/StringBuilder.append(Ljava/lang/StringBuffer;)Ljava/lang/StringBuilder
java/lang/StringBuilder.append(Ljava/lang/CharSequence;)Ljava/lang/StringBuilder;:0,1#1
java/lang/StringBuilder.append(Ljava/lang/CharSequence;II)Ljava/lang/StringBuilder;:2,3#3
java/lang/StringBuilder.append(Z)Ljava/lang/StringBuilder;:1
java/lang/StringBuilder.append(C)Ljava/lang/StringBuilder;:1
java/lang/StringBuilder.append(C)Ljava/lang/StringBuilder;:0,1
java/lang/StringBuilder.append(D)Ljava/lang/StringBuilder;:2
java/lang/StringBuilder.append(F)Ljava/lang/StringBuilder;:1
java/lang/StringBuilder.append(I)Ljava/lang/StringBuilder;:1
Expand Down Expand Up @@ -121,7 +121,7 @@ java/lang/StringBuffer.append(Ljava/lang/StringBuffer;)Ljava/lang/StringBuffer;:
java/lang/StringBuffer.append(Ljava/lang/CharSequence;)Ljava/lang/StringBuffer;:0,1#1
java/lang/StringBuffer.append(Ljava/lang/CharSequence;II)Ljava/lang/StringBuffer;:2,3#3
java/lang/StringBuffer.append(Z)Ljava/lang/StringBuffer;:1
java/lang/StringBuffer.append(C)Ljava/lang/StringBuffer;:1
java/lang/StringBuffer.append(C)Ljava/lang/StringBuffer;:0,1
java/lang/StringBuffer.append(D)Ljava/lang/StringBuffer;:2
java/lang/StringBuffer.append(F)Ljava/lang/StringBuffer;:1
java/lang/StringBuffer.append(I)Ljava/lang/StringBuffer;:1
Expand Down Expand Up @@ -177,7 +177,7 @@ java/lang/String.toUpperCase()Ljava/lang/String;:0
java/lang/String.toUpperCase(Ljava/util/Locale;)Ljava/lang/String;:1
java/lang/String.trim()Ljava/lang/String;:0
java/lang/String.valueOf(Z)Ljava/lang/String;:SAFE
java/lang/String.valueOf(C)Ljava/lang/String;:SAFE
java/lang/String.valueOf(C)Ljava/lang/String;:0
java/lang/String.valueOf(D)Ljava/lang/String;:SAFE
java/lang/String.valueOf(F)Ljava/lang/String;:SAFE
java/lang/String.valueOf(I)Ljava/lang/String;:SAFE
Expand All @@ -193,9 +193,9 @@ java/lang/CharSequence.subSequence(II)Ljava/lang/CharSequence;:2

java/lang/Boolean.toString()Ljava/lang/String;:SAFE
java/lang/Boolean.toString(B)Ljava/lang/String;:SAFE
java/lang/Character.getName(I)Ljava/lang/String;:SAFE
java/lang/Character.toString()Ljava/lang/String;:SAFE
java/lang/Character.toString(C)Ljava/lang/String;:SAFE
java/lang/Character.getName(I)Ljava/lang/String;:0
java/lang/Character.toString()Ljava/lang/String;:0
java/lang/Character.toString(C)Ljava/lang/String;:0
java/lang/Double.toString()Ljava/lang/String;:SAFE
java/lang/Double.toString(D)Ljava/lang/String;:SAFE
java/lang/Float.toHexString(F)Ljava/lang/String;:SAFE
Expand Down Expand Up @@ -258,4 +258,4 @@ java/util/ResourceBundle.getObject(Ljava/lang/String;)Ljava/lang/Object;:SAFE
kotlin/text/StringsKt.replace$default(Ljava/lang/String;CCZILjava/lang/Object;)Ljava/lang/String;:5
kotlin/text/StringsKt.replace$default(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZILjava/lang/Object;)Ljava/lang/String;:5
kotlin/text/Regex.replace(Ljava/lang/CharSequence;Ljava/lang/String;)Ljava/lang/String;:1
kotlin/text/Regex.<init>(Ljava/lang/String;)V:SAFE
kotlin/text/Regex.<init>(Ljava/lang/String;)V:SAFE
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/**
* Find Security Bugs
* Copyright (c) Philippe Arteau, All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3.0 of the License, or (at your option) any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this library.
*/
package com.h3xstream.findsecbugs.taintanalysis;

import com.h3xstream.findbugs.test.BaseDetectorTest;
import com.h3xstream.findbugs.test.EasyBugReporter;
import org.testng.annotations.Test;

import static org.testng.Assert.assertTrue;

import static org.mockito.Mockito.*;

public class CharacterTaintPropagationTest extends BaseDetectorTest {

@Test
public void validateCharacterTaintPropagation() throws Exception {
//Locate test code
String[] files = { getClassFilePath("testcode/taint/CharacterTaintPropagation") };

//Run the analysis
EasyBugReporter reporter = spy(new SecurityReporter());

analyze(files, reporter);

verify(reporter, times(9)).doReportBug(
bugDefinition().bugType("PATH_TRAVERSAL_IN").build());

verify(reporter, times(1)).doReportBug(
bugDefinition().bugType("PATH_TRAVERSAL_IN")
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromCharConcat")
.withPriority("Medium")
.build());

verify(reporter, times(1)).doReportBug(
bugDefinition().bugType("PATH_TRAVERSAL_IN")
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromChar")
.withPriority("Medium")
.build());

verify(reporter, times(1)).doReportBug(
bugDefinition().bugType("PATH_TRAVERSAL_IN")
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromCharToString")
.withPriority("Medium")
.build());

verify(reporter, times(1)).doReportBug(
bugDefinition().bugType("PATH_TRAVERSAL_IN")
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromCharGetName")
.withPriority("Medium")
.build());

verify(reporter, times(1)).doReportBug(
bugDefinition().bugType("PATH_TRAVERSAL_IN")
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromCharStringBuilder")
.withPriority("Medium")
.build());

verify(reporter, times(1)).doReportBug(
bugDefinition().bugType("PATH_TRAVERSAL_IN")
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromCharStringBuffer")
.withPriority("Medium")
.build());

verify(reporter, times(1)).doReportBug(
bugDefinition().bugType("PATH_TRAVERSAL_IN")
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromCharacterToString")
.withPriority("Medium")
.build());

verify(reporter, times(1)).doReportBug(
bugDefinition().bugType("PATH_TRAVERSAL_IN")
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromCharacterConcat")
.withPriority("Medium")
.build());

verify(reporter, times(1)).doReportBug(
bugDefinition().bugType("PATH_TRAVERSAL_IN")
.inClass("CharacterTaintPropagation").inMethod("unsafeFileFromString")
.withPriority("Medium")
.build());

verify(reporter, never()).doReportBug(
bugDefinition().bugType("PATH_TRAVERSAL_IN")
.inClass("CharacterTaintPropagation").inMethod("safeFileFromConstant")
.build());

verify(reporter, never()).doReportBug(
bugDefinition().bugType("PATH_TRAVERSAL_IN")
.inClass("CharacterTaintPropagation").inMethod("safeFileFromConstantWithInt")
.build());

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package testcode.taint;

import java.io.FileInputStream;
import java.io.FileNotFoundException;

public class CharacterTaintPropagation {

private static final String SAFE_PART = "staticSafePart";

/** This should report PATH_TRAVERSAL_IN. */
public FileInputStream unsafeFileFromCharConcat(final char unsafePart) throws FileNotFoundException {
return new FileInputStream("safePart" + unsafePart);
}

/** This should report PATH_TRAVERSAL_IN. */
public FileInputStream unsafeFileFromChar(final char unsafePart) throws FileNotFoundException {
return new FileInputStream(String.valueOf(unsafePart));
}

/** This should report PATH_TRAVERSAL_IN. */
public FileInputStream unsafeFileFromCharToString(final char unsafePart) throws FileNotFoundException {
return new FileInputStream(Character.toString(unsafePart));
}

/** This should report PATH_TRAVERSAL_IN. */
public FileInputStream unsafeFileFromCharGetName(final int unsafePart) throws FileNotFoundException {
return new FileInputStream(Character.getName(unsafePart));
}

/** This should report PATH_TRAVERSAL_IN. */
public FileInputStream unsafeFileFromCharStringBuilder(final char unsafePart) throws FileNotFoundException {
return new FileInputStream(new StringBuilder("safePart").append(unsafePart).toString());
}

/** This should report PATH_TRAVERSAL_IN. */
public FileInputStream unsafeFileFromCharStringBuffer(final char unsafePart) throws FileNotFoundException {
return new FileInputStream(new StringBuffer("safePart").append(unsafePart).toString());
}

/** This should report PATH_TRAVERSAL_IN. */
public FileInputStream unsafeFileFromCharacterToString(final Character unsafePart) throws FileNotFoundException {
return new FileInputStream(unsafePart.toString());
}

/** This should report PATH_TRAVERSAL_IN. */
public FileInputStream unsafeFileFromCharacterConcat(final Character unsafePart) throws FileNotFoundException {
return new FileInputStream("safePart" + unsafePart);
}

/** This should report PATH_TRAVERSAL_IN. */
public FileInputStream unsafeFileFromString(final String unsafePart) throws FileNotFoundException {
return new FileInputStream("safePart" + unsafePart);
}

public FileInputStream safeFileFromConstant() throws FileNotFoundException {
return new FileInputStream(SAFE_PART);
}

public FileInputStream safeFileFromConstantWithInt(final int safeInt) throws FileNotFoundException {
return new FileInputStream(new StringBuilder(SAFE_PART).append(safeInt).toString());
}
}

0 comments on commit 0ca18b5

Please sign in to comment.