Skip to content

Commit

Permalink
Merge pull request #604 from h3xstream/master
Browse files Browse the repository at this point in the history
New samples with StringSubstitutor were added #538
  • Loading branch information
h3xstream committed Oct 6, 2020
2 parents cf2e66a + d0d9e61 commit 246ed77
Show file tree
Hide file tree
Showing 7 changed files with 306 additions and 0 deletions.
Expand Up @@ -184,6 +184,7 @@ java/lang/String.valueOf(I)Ljava/lang/String;:SAFE
java/lang/String.valueOf(J)Ljava/lang/String;:SAFE
java/lang/String.valueOf(Ljava/lang/Object;)Ljava/lang/String;:0
java/lang/String.valueOf([C)Ljava/lang/String;:0
java/lang/String.toCharArray()[C:0

java/util/Arrays.toString([Ljava/lang/Object;)Ljava/lang/String;:0

Expand Down
25 changes: 25 additions & 0 deletions findsecbugs-plugin/src/main/resources/taint-config/other.txt
Expand Up @@ -63,3 +63,28 @@ flexjson/JSONSerializer.serialize(Ljava/lang/Object;Ljava/lang/StringBuilder;)Lj
javax/jdo/PersistenceManager.newQuery(Ljava/lang/Object;)Ljavax/jdo/Query;:UNKNOWN
javax/jdo/PersistenceManager.newQuery(Ljava/lang/String;Ljava/lang/Object;)Ljavax/jdo/Query;:UNKNOWN

--StringSubstitutor

org/apache/commons/text/StringSubstitutor.<init>()V:SAFE
org/apache/commons/text/StringSubstitutor.<init>(Ljava/util/Map;)V:0#1,2
org/apache/commons/text/StringSubstitutor.<init>(Ljava/util/Map;Ljava/lang/String;Ljava/lang/String;)V:0,1,2#3,4
org/apache/commons/text/StringSubstitutor.<init>(Ljava/util/Map;Ljava/lang/String;Ljava/lang/String;C)V:1,2,3#4,5
org/apache/commons/text/StringSubstitutor.<init>(Ljava/util/Map;Ljava/lang/String;Ljava/lang/String;CLjava/lang/String;)V:2,3,4#5,6

org/apache/commons/text/StringSubstitutor.replace(Ljava/lang/String;)Ljava/lang/String;:0,1#1
org/apache/commons/text/StringSubstitutor.replace(Ljava/lang/Object;)Ljava/lang/String;:0,1#1
org/apache/commons/text/StringSubstitutor.replace(Ljava/lang/CharSequence;)Ljava/lang/String;:0,1#1
org/apache/commons/text/StringSubstitutor.replace([C)Ljava/lang/String;:0,1#1,2
org/apache/commons/text/StringSubstitutor.replace(Ljava/lang/Object;Ljava/util/Map;)Ljava/lang/String;:0,1#0

org/apache/commons/lang3/text/StrSubstitutor.<init>()V:SAFE
org/apache/commons/lang3/text/StrSubstitutor.<init>(Ljava/util/Map;)V:0#1,2
org/apache/commons/lang3/text/StrSubstitutor.<init>(Ljava/util/Map;Ljava/lang/String;Ljava/lang/String;)V:0,1,2#3,4
org/apache/commons/lang3/text/StrSubstitutor.<init>(Ljava/util/Map;Ljava/lang/String;Ljava/lang/String;C)V:1,2,3#4,5
org/apache/commons/lang3/text/StrSubstitutor.<init>(Ljava/util/Map;Ljava/lang/String;Ljava/lang/String;CLjava/lang/String;)V:2,3,4#5,6

org/apache/commons/lang3/text/StrSubstitutor.replace(Ljava/lang/String;)Ljava/lang/String;:0,1#1
org/apache/commons/lang3/text/StrSubstitutor.replace(Ljava/lang/Object;)Ljava/lang/String;:0,1#1
org/apache/commons/lang3/text/StrSubstitutor.replace(Ljava/lang/CharSequence;)Ljava/lang/String;:0,1#1
org/apache/commons/lang3/text/StrSubstitutor.replace([C)Ljava/lang/String;:0,1#1,2
org/apache/commons/lang3/text/StrSubstitutor.replace(Ljava/lang/Object;Ljava/util/Map;)Ljava/lang/String;:0,1#0
@@ -0,0 +1,73 @@
/**
* 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;

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

import static org.mockito.Mockito.*;

public class StringSubstitutorTest extends BaseDetectorTest {

@Test
public void stringSubstitutorUnsafe() throws Exception {
//Locate test code
String[] files = {
getClassFilePath("testcode/strsubstitutor/StringSubstitutorUnsafe")
};

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

verify(reporter,times(2)).doReportBug(bugDefinition()
.bugType("SQL_INJECTION_SPRING_JDBC").inClass("StringSubstitutorUnsafe").inMethod("addUser1Unsafe")
.build());
verify(reporter).doReportBug(bugDefinition()
.bugType("SQL_INJECTION_SPRING_JDBC").inClass("StringSubstitutorUnsafe").inMethod("addUser2Unsafe")
.build());
verify(reporter,times(2)).doReportBug(bugDefinition()
.bugType("SQL_INJECTION_SPRING_JDBC").inClass("StringSubstitutorUnsafe").inMethod("addUser3Unsafe")
.build());

//Total number is 5
verify(reporter,times(5)).doReportBug(bugDefinition()
.bugType("SQL_INJECTION_SPRING_JDBC").inClass("StringSubstitutorUnsafe").build());
}


@Test
public void stringSubstitutorSafe() throws Exception {
//FindSecBugsGlobalConfig.getInstance().setDebugPrintInvocationVisited(true);

//Locate test code
String[] files = {
getClassFilePath("testcode/strsubstitutor/StringSubstitutorSafe")
};

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

verify(reporter,never()).doReportBug(bugDefinition()
.bugType("SQL_INJECTION_SPRING_JDBC").inClass("StringSubstitutorSafe")
.build());
}

}
@@ -0,0 +1,50 @@
package org.apache.commons.lang3.text;

import java.util.Map;

/**
* Changes might needs synchronisation with org.apache.commons.text.StringSubstitutor
*/
public class StrSubstitutor {

public StrSubstitutor() {

}

public <V> StrSubstitutor(final Map<String, V> valueMap) {

}

public <V> StrSubstitutor(Map<String, V> valueMap, String prefix, String suffix) {

}

public <V> StrSubstitutor(final Map<String, V> valueMap, final String prefix, final String suffix,
final char escape) {
}


public <V> StrSubstitutor(final Map<String, V> valueMap, final String prefix, final String suffix,
final char escape, final String valueDelimiter) {

}


public static <V> String replace(final Object source, final Map<String, V> valueMap) {
return null;
}


public String replace(final char[] source) {
return null;
}
public String replace(final CharSequence source) {
return null;
}
public String replace(final Object source) {
return null;
}
public String replace(final String source) {
return null;
}
}
@@ -0,0 +1,50 @@
package org.apache.commons.text;

import java.util.Map;

/**
* Changes might needs synchronisation with org.apache.commons.lang3.text.StringSubstitutor
*/
public class StringSubstitutor {

public StringSubstitutor() {

}

public <V> StringSubstitutor(final Map<String, V> valueMap) {

}

public <V> StringSubstitutor(Map<String, V> valueMap, String prefix, String suffix) {

}

public <V> StringSubstitutor(final Map<String, V> valueMap, final String prefix, final String suffix,
final char escape) {
}


public <V> StringSubstitutor(final Map<String, V> valueMap, final String prefix, final String suffix,
final char escape, final String valueDelimiter) {

}


public static <V> String replace(final Object source, final Map<String, V> valueMap) {
return null;
}


public String replace(final char[] source) {
return null;
}
public String replace(final CharSequence source) {
return null;
}
public String replace(final Object source) {
return null;
}
public String replace(final String source) {
return null;
}
}
@@ -0,0 +1,53 @@
package testcode.strsubstitutor;

import org.apache.commons.text.StringSubstitutor;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.jdbc.core.JdbcTemplate;

import java.util.HashMap;
import java.util.Map;

@SpringBootApplication
public class StringSubstitutorSafe {

@Autowired
JdbcTemplate jdbcTemplate;

public static final String TEMPLATE_STRING = "";


public void addUser1Safe(String templateInput) {

Map<String, String> valuesMap = new HashMap<>();
valuesMap.put("animal", "quick brown fox");
valuesMap.put("target", "lazy dog");

StringSubstitutor str = new StringSubstitutor(valuesMap); //valuesMap is considered safe

String templateString = "The ${animal} jumped over the ${target}.";
jdbcTemplate.execute(str.replace(templateString)); //StringSubstitutor and templateString are safe (templateInput)
}

public void staticSubstitutor(Map<String, String> valuesMap,String input) {
String result = StringSubstitutor.replace("",new HashMap<String, String>());

jdbcTemplate.execute(result);
}

public void otherStringSubstitutorSignature() {
StringSubstitutor str1 = new StringSubstitutor(new HashMap<String, String>());
StringSubstitutor str2 = new StringSubstitutor(new HashMap<String, String>(),"","");
StringSubstitutor str3 = new StringSubstitutor(new HashMap<String, String>(),"","",'\\');
StringSubstitutor str4 = new StringSubstitutor(new HashMap<String, String>(),"","",'\\', ":-");

String templateString = "The ${animal} jumped over the ${target}.";
jdbcTemplate.execute(str1.replace(templateString));
jdbcTemplate.execute(str2.replace((Object) templateString));
jdbcTemplate.execute(str3.replace(new StringBuilder(templateString)));
jdbcTemplate.execute(str4.replace(templateString.toCharArray()));
}



}
@@ -0,0 +1,54 @@
package testcode.strsubstitutor;

import org.apache.commons.text.StringSubstitutor;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.jdbc.core.JdbcTemplate;

import java.util.HashMap;
import java.util.Map;

@SpringBootApplication
public class StringSubstitutorUnsafe {

@Autowired
JdbcTemplate jdbcTemplate;

public static final String TEMPLATE_STRING = "";


public void addUser1Unsafe(String templateInput) {

Map<String, String> valuesMap = new HashMap<>();
valuesMap.put("animal", "quick brown fox");
valuesMap.put("target", "lazy dog");

StringSubstitutor str = new StringSubstitutor(valuesMap); //valuesMap is considered safe

jdbcTemplate.execute(str.replace(templateInput)); //Base template is unsafe (templateInput)
jdbcTemplate.execute(templateInput); //Just to make sure the API is cover
}

public void addUser2Unsafe(String someInput) {

Map<String, String> valuesMap = new HashMap<>();
valuesMap.put("animal", "quick brown fox");
valuesMap.put("target", someInput);

StringSubstitutor str = new StringSubstitutor(valuesMap); //The map is tainted

String templateString = "The ${animal} jumped over the ${target}.";
jdbcTemplate.execute(str.replace(templateString)); //StringSubstitutor is unsafe and should remain.
}

public void addUser3Unsafe(String someInput) {
String result1 = StringSubstitutor.replace(someInput,new HashMap<String, String>()); //Template string is from unknown source

Map<String, String> valuesMap = new HashMap<String, String>();
valuesMap.put("test",someInput);
String result2 = StringSubstitutor.replace("",valuesMap); //Map string is from unknown source

jdbcTemplate.execute(result1);
jdbcTemplate.execute(result2);
}
}

0 comments on commit 246ed77

Please sign in to comment.