Unsafe string comparison

Comparing strings, if not done properly, can be dangerous, too. Only checking that a string contains a certain value rather than checking for equality can leave inputs open to exploitation. Normally this is not something worth mentioning, but these are often easily and accidentally confused. This can happen any time strings are compared, whether with regular expressions or traditional comparisons.

The vulnerability

The below examples are unsafe because 'admin123' is still still valid, as 'admin' still occurs in the string. This would allow "admin123" to access features that should only be available to "admin":

   >>> username = "admin123"
   >>> if "admin" in username:
   ...   print("User is an admin")
   'User is an admin'
   

These functions check to see if the search string occurs at all within a string, but do not properly check equality between the two. When on a programming binge (hours and hours of coding), developers often overlook simple flaws like this.

• PHP:

     if (strstr("admin", $username)) ... 

• Python:

     if "admin" in username: ...

• Ruby:

     if username.include? "admin" ...
     # OR
     # if username["admin"] ...

• Perl:

     if (index($username, "admin") != -1) ...

• An example using regular expressions:

   >>> username = "admin"
   >>> if re.match("admin", username): print("admin")
   ...
   'admin'

Countermeasures

Mitigation is rather simple, make sure that comparison operators are used instead of find functions. Be extra careful to avoid accidental assignment during input checks.

• PHP:

     if ($username == "admin") ... 

• Python:

     if username == "admin": ...

• Ruby:

     if  username == 'admin': ...

• Perl:

     if ($username eq 'admin') ...

All regular expressions used for absolute comparison should be all inclusive. This means that the first character of the pattern should be '^', indicating the beginning of the string, while the last character should be '$', indicating the end of the string.

   >>> pattern = '^admin$'
   >>> username = "admin123"
   >>> if re.match(pattern, username): print("is an admin")
   ...
   

Unless the developer is explicity searching for the position or index of a substring or character within the target or trying to determine its presence in the target string, find functions should not be used and all regular expressions must be exact (with the exception of case insensitivity).

Observed in the wild:

     Wordpress timthumb.php domain name handling exploit (2011) leveraged this type of vulnerability.  
     Secunia Advisory 45416 https://secunia.com/advisories/45416/

Auditing

In order to audit this, a developer needs to identify all uses of find functions and verify that the usage is safe. A search can be done using:

        find -name \*.py -exec grep -HnC2 \\bin\\b '{}' \; \
               -o -name \*.php\* -exec grep -HnC2 str.\?str\( '{}' \; \
               -o -name \*.rb -exec grep -HnC2 \\.include? '{}' \; \
               -o -regextype posix-awk -regex ".*\.(pl|pm)"  -exec grep -HnC2 \\bindex\( '{}' \; &>  string_comparison.txt