Merge pull request #7574 from pwntester/improve_strings_qll

Add models for AbstractStringBuilder.substring,subsequence,getChars

Author: atorralba

java/ql/lib/semmle/code/java/frameworks/Strings.qll

@@ -46,11 +46,14 @@ private class StringSummaryCsv extends SummaryModelCsv {
         "java.lang;AbstractStringBuilder;true;AbstractStringBuilder;(String);;Argument[0];Argument[-1];taint",
         "java.lang;AbstractStringBuilder;true;append;;;Argument[0];Argument[-1];taint",
         "java.lang;AbstractStringBuilder;true;append;;;Argument[-1];ReturnValue;value",
+        "java.lang;AbstractStringBuilder;true;getChars;;;Argument[-1];Argument[2];taint",
         "java.lang;AbstractStringBuilder;true;insert;;;Argument[1];Argument[-1];taint",
         "java.lang;AbstractStringBuilder;true;insert;;;Argument[-1];ReturnValue;value",
         "java.lang;AbstractStringBuilder;true;replace;;;Argument[-1];ReturnValue;value",
         "java.lang;AbstractStringBuilder;true;replace;;;Argument[2];Argument[-1];taint",
         "java.lang;AbstractStringBuilder;true;reverse;;;Argument[-1];ReturnValue;value",
+        "java.lang;AbstractStringBuilder;true;subSequence;;;Argument[-1];ReturnValue;taint",
+        "java.lang;AbstractStringBuilder;true;substring;;;Argument[-1];ReturnValue;taint",
         "java.lang;AbstractStringBuilder;true;toString;;;Argument[-1];ReturnValue;taint",
         "java.lang;StringBuffer;true;StringBuffer;(CharSequence);;Argument[0];Argument[-1];taint",
         "java.lang;StringBuffer;true;StringBuffer;(String);;Argument[0];Argument[-1];taint",

java/ql/test/library-tests/dataflow/taint/StringBuilderTests.java

@@ -63,4 +63,27 @@ static void stringBuilderInsertBad() {
     sb.insert(45, taint());
     sink(sb.toString());
   }
+
+  static void stringBuilderGetCharsBad() {
+    StringBuilder sb = new StringBuilder();
+    sb.append("from preferences select locale where user=''");
+    sb.append(taint());
+    char[] chars = null;
+    sb.getChars(0, 0, chars, 0);
+    sink(new String(chars));
+  }
+
+  static void stringBuilderSubSequenceBad() {
+    StringBuilder sb = new StringBuilder();
+    sb.append("from preferences select locale where user=''");
+    sb.append(taint());
+    sink(sb.subSequence(0, 0).toString());
+  }
+
+  static void stringBuilderSubstringBad() {
+    StringBuilder sb = new StringBuilder();
+    sb.append("from preferences select locale where user=''");
+    sb.append(taint());
+    sink(sb.substring(0, 0));
+  }
 }

java/ql/test/library-tests/dataflow/taint/test.expected

@@ -56,6 +56,9 @@
 | StringBuilderTests.java:48:69:48:75 | taint(...) | StringBuilderTests.java:50:10:50:22 | toString(...) |
 | StringBuilderTests.java:56:24:56:30 | taint(...) | StringBuilderTests.java:57:10:57:22 | toString(...) |
 | StringBuilderTests.java:63:19:63:25 | taint(...) | StringBuilderTests.java:64:10:64:22 | toString(...) |
+| StringBuilderTests.java:70:15:70:21 | taint(...) | StringBuilderTests.java:73:10:73:26 | new String(...) |
+| StringBuilderTests.java:79:15:79:21 | taint(...) | StringBuilderTests.java:80:10:80:40 | toString(...) |
+| StringBuilderTests.java:86:15:86:21 | taint(...) | StringBuilderTests.java:87:10:87:27 | substring(...) |
 | Varargs.java:7:8:7:14 | taint(...) | Varargs.java:14:10:14:10 | s |
 | Varargs.java:8:8:8:14 | taint(...) | Varargs.java:19:10:19:10 | s |
 | Varargs.java:8:17:8:23 | taint(...) | Varargs.java:19:10:19:10 | s |