Page History

...

This noncompliant code example searches periodically loads a log file of previous searches for keywords that match a regular expression to present search suggestions to the user. The suggestSearches() method is repeatedly called to provide suggestions for the user for completion of the search text. The full log of previous searches is stored in the logBuffer object. The strings in logBuffer are periodically copied to the log object for use in searchSuggestions().to memory and allows clients to obtain keyword search suggestions by passing the keyword as an argument to suggestSearches().

Code Block

bgColor	#FFCCCC

import java.util.HashSet;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public final class ExploitableLog {public class Keywords {
  private static ScheduledExecutorService scheduler = Executors
      .newSingleThreadScheduledExecutor();
  private static CharBuffer log;
  private static final StringBuilderObject logBufferlock = new StringBuilderObject();

  privatepublic static Set<String> suggestSearches(String log = logBuffer.toString();

  static {
    // this is supposed to come from a file, but its here as a string for search) {
    synchronized(lock) {
      Set<String> searches = new HashSet<String>();

      // Construct regex dynamically from user string
    // illustrative purposes.String Eachregex line's format is: name,id,timestamp
    append("Alice,1267773881,2147651408\n");
    append("Bono,1267774881,2147351708\n"= "(" + search + ".*),[\\d]+?,[\\d]+?";
  
      Pattern keywordPattern = Pattern.compile(regex);
    append("Charles,1267775881,1175523058\n");
    append("Cecilia,1267773222,291232332\n"  Matcher logMatcher = keywordPattern.matcher(log);
  }
    while (logMatcher.find()) {
  private static void append(CharSequence str) {
 String found = logBufferlogMatcher.appendgroup(str1);
    log   = logBuffersearches.toStringadd(found);
      }
      return searches;
 // update log string} on append
  }

  static public{
 static Set<String> suggestSearches(String search)try {
    Set<String>  FileChannel searcheschannel = new HashSet<String> FileInputStream(
          "path").getChannel();

      // Construct regex from user string
 Get the file's size and map it into memory
      int size = (int) channel.size();
      Stringfinal MappedByteBuffer regexmappedBuffer = "^(" + search + ".*),[0-9]+?,[0-9]+?$";
channel.map(
          FileChannel.MapMode.READ_ONLY, 0, size);

      intCharset flagscharset = Pattern.MULTILINE;
Charset.forName("ISO-8859-15");
      final PatternCharsetDecoder keywordPatterndecoder = Patterncharset.compilenewDecoder(regex, flags);

      log = decoder.decode(mappedBuffer); // Match regex
 Read file into char buffer

     Matcher Runnable logMatcherperiodicLogRead = new keywordPattern.matcherRunnable(log); {
    while (logMatcher.find())    @Override public void run() {
          synchronized(lock) { 
            try {
      String found        log = logMatcher.group(1)decoder.decode(mappedBuffer);
            } catch (CharacterCodingException e) {
              // Forward to handler 
            } 
          }
        }
      };
      searchesscheduler.add(foundscheduleAtFixedRate(periodicLogRead, 0, 5, TimeUnit.SECONDS);
    } catch (Throwable t) {
      // Forward to handler
    return searches;}
  }
}

The regex used to search the log islog file is parsed using a regex constructed dynamically from user input (search):

No Format
^^"(" + search + ".*),[0-9\\d]+?,[0-9\\d]+?$"

This regex matches against an entire line of the log and searches for old searches beginning with the entered keyword. The anchoring operators and use of the reluctance operators mitigate some greediness concerns. The grouping characters allow the program to grab only the keyword while still matching the IP and timestamp. Because the log String contains multiple lines, the MULTILINE flag must be active to force the anchoring operators to match against newlines. By all appearances, this is a strong regex.

However, this class does not sanitize the incoming regular expression, and as a result, exposes too much information from the log file to the user.

finds lines in the log that correspond to the value of search. Consequently, if an attacker can perform regex injection, sensitive information such as the IP Address of the user may be disclosed.

A trusted user might enter the "C" keyword A non-malicious use of the searchSuggestions() method would be to enter "C" to match "Charles" and "Cecilia". However, however, a malicious user could enter

No Format
?:)(^.*,[0-9]+?,[0-9]+?$)\|(?:

which grabs the entire log line rather than just the old keywords. The first close parentheses of the malicious search string defeats the grouping protection. Using the OR the string .*)|(. This might result in unintended disclosure of sensitive information present in the log. Using the OR (|) operator allows injection of any arbitrary regex. Now this regex will reveal all IPs and timestamps of past searches.

Compliant Solution

One method of preventing mitigating this vulnerability is to filter out the sensitive information prior to matching and then running the user-supplied regex against the remaining non-sensitive information. However, sensitive information may be exposed 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.)but the class is not refactored to accommodate the changes.

Compliant Solution

This compliant solution filters out non-alphanumeric characters (except space and single quote) from the search string using Java's Character.isLetterOrDigit(). This removes the grouping parentheses and the OR operator which triggers the , which prevents regex injection.

Code Block

bgColor	#ccccff


import java.util.HashSet;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public final class FilteredLogKeywords {
  // ...

  public static Set<String> suggestSearches(String search) {
    synchronized(lock) {
      Set<String> searches = new HashSet<String>();

    // Filter bad chars from 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) ||
            ch == ' ' ||
            ch == '\'') {
          sb.append(ch);
        }
      }
      search = sb.toString();

      // Construct regex dynamically from user string
      String regex = "^(" + search + ".*),[0-9\\d]+?,[0-9\\d]+?$";
    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;
  }
}
// ...
}

Risk Assessment

Violating this guideline may result in the disclosure of sensitive information disclosure.

Rule	Severity	Liklihood	Remediation Cost	Priority	Level
IDS18- J	medium	probable unlikely	high medium	P8 P4	L2 L3

References

Wiki Markup

\[[Tutorials 08|AA. Java References#Tutorials 08]\] [Regular Expressions|http://java.sun.com/docs/books/tutorial/essential/regex/index.html]
\[[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]

Space shortcuts

Page tree

Versions Compared

Old Version 39

New Version 40

Key

Compliant Solution

Compliant Solution

Risk Assessment

References