...
This noncompliant code example searches a log file of previous searches for keywords that match a regular expression to present search suggestions to the user. The function suggestSearches() is repeatedly called to bring up suggestions for the user for auto-completion. The full log of previous searches is stored in the logBuffer StringBuffer object. StringBuffer is typical for this use-case because appends are relatively expensive on Strings but cheaper on StringBuilders and StringBuffers. Furthermore, the StringBuffer class is thread-safe. StringBuilder object. The strings in logBuffer are periodically copied to the log String object for use in searchSuggestions().
...
A non-malicious use of the searchSuggestions() method would be to enter "C" to match "Charles" and "Cecilia". However, a malicious user could enter
| No Format |
|---|
?:)(^C^.*,[0-9]+?,[0-9]+?$)|(?: |
which grabs the entire log line rather than just the old keywords. The outer parentheses of the malicious search string defeat the grouping protection. Using the OR operator allows injection of any arbitrary regex. Now this use will reveal all times and IPs the keyword 'C' was searchedof past searches.
| Code Block | ||
|---|---|---|
| ||
import java.util.HashSet; import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; public final class ExploitableLog {    private static final StringBufferStringBuilder logBuffer = new StringBufferStringBuilder();    private static String log = logBuffer.toString();       public static Set<String> suggestSearches(String search) {       Set<String> searches = new HashSet<String>();             // Construct regex from user string       String regex = "^(" + search + ".*),[0-9]+?,[0-9]+?$";       int flags = Pattern.MULTILINE;       Pattern keywordPattern = Pattern.compile(regex, flags);             // Match regex       Matcher logMatcher = keywordPattern.matcher(log);       while (logMatcher.find()) {          String found = logMatcher.group(1);          searches.add(found);       }             return searches;    }       public private static void append(CharSequence str) {       logBuffer.append(str);       log = logBuffer.toString(); //update log string on append    }    static {       // this is supposed to come from a file, but its here as a string for       // illustrative purposes       append("Alice,1267773881,2147651408\n");       append("Bono,1267774881,2147351708\n");       append("Charles,1267775881,1175523058\n");       append("Cecilia,1267773222,291232332\n");    } } |
Compliant Solution
One somewhat compliant solutions solution is to parse out the sensitive information prior to matching and then running the user-supplied regex against that. Depending However, if the log format changes without a corresponding change in the class, sensitive information may be exposed. Furthermore, depending on how encapsulated the search keywords are, a malicious user may be able to grab a list of all the keywords (If there are a lot of keywords, this may cause a denial of service).
Alternatively, whitelisting certain characters (such as letters and digits) and then passing the filtered keyword onto the regex mitigates the vulnerability. Blacklisting might be difficult due to the variability of the regex language.
...
| Code Block | ||
|---|---|---|
| ||
import java.util.HashSet; import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; public final class FilteredLog {    private static StringBufferfinal StringBuilder logBuffer = new StringBufferStringBuilder();    private static String log = logBuffer.toString();       public static Set<String> suggestSearches(String search) {       Set<String> searches = new HashSet<String>();             // Filter user input       StringBuilder sb = new StringBuilder(search.length());       for (int i = 0; i < search.length(); ++i) {          char ch = search.charAt(i);          if (Character.isLetterOrDigit(ch))             sb.append(ch);       }       search = sb.toString();             // Construct regex from user string       String regex = "^(" + search + ".*),[0-9]+?,[0-9]+?$";       int flags = Pattern.MULTILINE;       Pattern keywordPattern = Pattern.compile(regex, flags);             // Match regex       Matcher logMatcher = keywordPattern.matcher(log);       while (logMatcher.find()) {          String found = logMatcher.group(1);          searches.add(found);       }             return searches;    }       public private static void append(CharSequence str) {       logBuffer.append(str);       log = logBuffer.toString(); //update log string on append    }    static {       // this is supposed to come from a file, but its here as a string for       // illustrative purposes       append("Alice,1267773881,2147651408\n");       append("Bono,1267774881,2147351708\n");       append("Charles,1267775881,1175523058\n");       append("Cecilia,1267773222,291232332\n");    } } |
...
Rule | Severity | Liklihood | Remediation Cost | Priority | Level |
|---|---|---|---|---|---|
IDS18-J | medium | probable | high | P8 | L2 |
Violating this guideline may result in sensitive information disclosure.
References
| Wiki Markup |
|---|
\[[MITRE 09|AA. Java References#MITRE 09]\] [CWE ID 625|http://cwe.mitre.org/data/definitions/625.html] "Permissive Regular Expressions" \[[CVE 05|AA. Java References#CVE]\] [CVE-2005-1949|http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2005-1949] |