hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-28 18:25:24 +02:00
Code Issues Packages Projects Releases Wiki Activity

Make taint flow through java.lang.String.(replace|replaceFirst|replaceAll) more permissive.

Browse Source
This commit is contained in:
Sebastian Bauersfeld
2022-08-18 15:41:10 +07:00
parent 5cf320d553
commit 354a7fd252
3 changed files with 5 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
View File

@@ -251,7 +251,7 @@ private predicate qualifierToArgumentStep(Expr tracked, Expr sink) {
/** Access to a method that passes taint from the qualifier. */
private predicate qualifierToMethodStep(Expr tracked, MethodAccess sink) {
(taintPreservingQualifierToMethod(sink.getMethod()) or unsafeEscape(sink)) and
taintPreservingQualifierToMethod(sink.getMethod()) and
tracked = sink.getQualifier()
}
@@ -282,28 +282,6 @@ private predicate taintPreservingQualifierToMethod(Method m) {
)
}
private class StringReplaceMethod extends TaintPreservingCallable {
StringReplaceMethod() {
this.getDeclaringType() instanceof TypeString and
(
this.hasName("replace") or
this.hasName("replaceAll") or
this.hasName("replaceFirst")
)
}
override predicate returnsTaintFrom(int arg) { arg = 1 }
}
private predicate unsafeEscape(MethodAccess ma) {
// Removing `<script>` tags using a string-replace method is
// unsafe if such a tag is embedded inside another one (e.g. `<scr<script>ipt>`).
exists(StringReplaceMethod m | ma.getMethod() = m |
ma.getArgument(0).(StringLiteral).getValue() = "(<script>)" and
ma.getArgument(1).(StringLiteral).getValue() = ""
)
}
/** Access to a method that passes taint from an argument. */
private predicate argToMethodStep(Expr tracked, MethodAccess sink) {
exists(Method m, int i |

3
java/ql/lib/semmle/code/java/frameworks/Strings.qll
View File

@@ -26,8 +26,11 @@ private class StringSummaryCsv extends SummaryModelCsv {
"java.lang;String;false;join;;;Argument[0..1];ReturnValue;taint;manual",
"java.lang;String;false;repeat;(int);;Argument[-1];ReturnValue;taint;manual",
"java.lang;String;false;replace;;;Argument[-1];ReturnValue;taint;manual",
"java.lang;String;false;replace;;;Argument[1];ReturnValue;taint;manual",
"java.lang;String;false;replaceAll;;;Argument[-1];ReturnValue;taint;manual",
"java.lang;String;false;replaceAll;;;Argument[1];ReturnValue;taint;manual",
"java.lang;String;false;replaceFirst;;;Argument[-1];ReturnValue;taint;manual",
"java.lang;String;false;replaceFirst;;;Argument[1];ReturnValue;taint;manual",
"java.lang;String;false;split;;;Argument[-1];ReturnValue;taint;manual",
"java.lang;String;false;String;;;Argument[0];Argument[-1];taint;manual",
"java.lang;String;false;strip;;;Argument[-1];ReturnValue;taint;manual",

2
java/ql/test/library-tests/dataflow/taint/B.java
View File

@@ -41,7 +41,7 @@ public class B {
String valueOfSubstring = String.valueOf(complex.toCharArray(), 0, 1);
sink(valueOfSubstring);
// tainted - unsafe escape
String badEscape = constructed.replaceAll("(<script>)", "");
String badEscape = constructed.replaceAll("irrelevant", "irrelevant");
sink(badEscape);
// tainted - tokenized string
String token = new StringTokenizer(badEscape).nextToken();