Questions about this topic? Sign up to ask in the talk tab.

Difference between revisions of "Unsafe string comparison"

From NetSec
Jump to: navigation, search
(Created page with "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 ex...")
 
(Countermeasures)
 
(2 intermediate revisions by the same user not shown)
Line 5: Line 5:
 
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":   
 
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"
+
{{code|text=<source lang="python">
    >>> if "admin" in username:
+
>>> username = "admin123"
    ...  print("User is an admin")
+
>>> if "admin" in username:
    'User is an admin'
+
...  print("User is an admin")
 +
'User is an admin'
 +
</source>}}
 
      
 
      
 
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.
 
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:
 
* PHP:
      if (strstr("admin", $username)) ...  
+
{{code|text=<source lang="php">
 +
if (strstr("admin", $username)) ...  
 +
</source>}}
 +
 
 
* Python:
 
* Python:
      if "admin" in username: ...
+
{{code|text=<source lang="python">
 +
if "admin" in username: ...
 +
</source>}}
 +
 
 
* Ruby:
 
* Ruby:
      if username.include? "admin" ...
+
{{code|text=<source lang="ruby">if username.include? "admin" ...
      # OR
+
# OR
      # if username["admin"] ...
+
# if username["admin"] ...
 +
</source>}}
 +
 
 
* Perl:
 
* Perl:
      if (index($username, "admin") != -1) ...
+
{{code|text=<source lang="perl">
 +
if (index($username, "admin") != -1) ...
 +
</source>}}
  
 
* An example using regular expressions:
 
* An example using regular expressions:
    >>> username = "admin"
+
{{code|text=<source lang="python">
    >>> if re.match("admin", username): print("admin")
+
>>> username = "admin"
    ...
+
>>> if re.match("admin", username): print("admin")
    'admin'
+
...
 +
'admin'
 +
</source>}}
  
 
== [[Countermeasures]] ==
 
== [[Countermeasures]] ==
Line 34: Line 48:
  
 
* PHP:
 
* PHP:
      if ($username == "admin") ...  
+
{{code|text=<source lang="php">
 +
if ($username == "admin") ...  
 +
</source>}}
 +
 
 
* Python:
 
* Python:
      if username == "admin": ...
+
{{code|text=<source lang="python">
 +
if username == "admin": ...
 +
</source>}}
 +
 
 
* Ruby:
 
* Ruby:
      if  username == 'admin': ...
+
{{code|text=<source lang="ruby">
* Perl:
+
if  username == 'admin': ...
      if ($username eq 'admin') ...
+
</source>}}
  
 +
* Perl:
 +
{{code|text=<source lang="perl">
 +
if ($username eq 'admin') ...
 +
</source>}}
  
 
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.
 
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$'
+
{{code|text=<source lang="python">
    >>> username = "admin123"
+
>>> pattern = '^admin$'
    >>> if re.match(pattern, username): print("is an admin")
+
>>> username = "admin123"
    ...
+
>>> if re.match(pattern, username): print("is an admin")
   
+
...
 +
</source>}}
 +
 
 
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).
 
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).
  
Line 58: Line 84:
 
== [[Auditing]] ==
 
== [[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:
 
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 '{}' \; \
+
{{code|text=<source lang="bash">
                -o -name \*.php\* -exec grep -HnC2 str.\?str\( '{}' \; \
+
find -name \*.py -exec grep -HnC2 \\bin\\b '{}' \; \
                -o -name \*.rb -exec grep -HnC2 \\.include? '{}' \; \
+
    -o -name \*.php\* -exec grep -HnC2 str.\?str\( '{}' \; \
                -o -regextype posix-awk -regex ".*\.(pl|pm)"  -exec grep -HnC2 \\bindex\( '{}' \; &>  string_comparison.txt
+
    -o -name \*.rb -exec grep -HnC2 \\.include? '{}' \; \
 +
    -o -regextype posix-awk -regex ".*\.(pl|pm)"  -exec grep -HnC2 \\bindex\( '{}' \; &>  string_comparison.txt
 +
</source>}}
 +
 
 
[[Category:Secure programming]]
 
[[Category:Secure programming]]

Latest revision as of 07:28, 6 December 2012

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