Incomplete multi-character sanitization

Sanitizing untrusted input is a common technique for preventing injection attacks and other security vulnerabilities. Regular expressions are often used to perform this sanitization. However, when the regular expression matches multiple consecutive characters, replacing it just once can result in the unsafe text reappearing in the sanitized input.

Attackers can exploit this issue by crafting inputs that, when sanitized with an ineffective regular expression, still contain malicious code or content. This can lead to code execution, data exposure, or other vulnerabilities.

Recommendation

To prevent this issue, it is highly recommended to use a well-tested sanitization library whenever possible. These libraries are more likely to handle corner cases and ensure effective sanitization.

If a library is not an option, you can consider alternative strategies to fix the issue. For example, applying the regular expression replacement repeatedly until no more replacements can be performed, or rewriting the regular expression to match single characters instead of the entire unsafe text.

Example

Consider the following JavaScript code that aims to remove all HTML comment start and end tags:

str.replace(/<!--|--!?>/g, "");   

Given the input string "<!<!--- comment --->>", the output will be "<!-- comment -->", which still contains an HTML comment.

One possible fix for this issue is to apply the regular expression replacement repeatedly until no more replacements can be performed. This ensures that the unsafe text does not re-appear in the sanitized input, effectively removing all instances of the targeted pattern:

function removeHtmlComments(input) {  
  let previous;  
  do {  
    previous = input;  
    input = input.replace(/<!--|--!?>/g, "");  
  } while (input !== previous);  
  return input;  
}  

Example

Another example is the following regular expression intended to remove script tags:

str.replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/g, "");  

If the input string is "<scrip<script>is removed</script>t>alert(123)</script>", the output will be "<script>alert(123)</script>", which still contains a script tag.

A fix for this issue is to rewrite the regular expression to match single characters ("<" and ">") instead of the entire unsafe text. This simplifies the sanitization process and ensures that all potentially unsafe characters are removed:

function removeAllHtmlTags(input) {  
  return input.replace(/<|>/g, "");  
}

Another potential fix is to use the popular sanitize-html npm library. It keeps most of the safe HTML tags while removing all unsafe tags and attributes.

const sanitizeHtml = require("sanitize-html");
function removeAllHtmlTags(input) {  
  return sanitizeHtml(input);  
}

Example

Lastly, consider a path sanitizer using the regular expression /\.\.\//:

str.replace(/\.\.\//g, "");  

The regular expression attempts to strip out all occurences of /../ from str. This will not work as expected: for the string /./.././, for example, it will remove the single occurrence of /../ in the middle, but the remainder of the string then becomes /../, which is another instance of the substring we were trying to remove.

A possible fix for this issue is to use the "sanitize-filename" npm library for path sanitization. This library is specifically designed to handle path sanitization, and should handle all corner cases and ensure effective sanitization:

const sanitize = require("sanitize-filename");  
  
function sanitizePath(input) {  
  return sanitize(input);  
}  

References

• OWASP Top 10: A1 Injection.
• Stack Overflow: Removing all script tags from HTML with JS regular expression.
• Common Weakness Enumeration: CWE-20.
• Common Weakness Enumeration: CWE-80.
• Common Weakness Enumeration: CWE-116.

Incomplete string escaping or encoding

Sanitizing untrusted input is a common technique for preventing injection attacks such as SQL injection or cross-site scripting. Usually, this is done by escaping meta-characters such as quotes in a domain-specific way so that they are treated as normal characters.

However, directly using the string replace method to perform escaping is notoriously error-prone. Common mistakes include only replacing the first occurrence of a meta-character, or backslash-escaping various meta-characters but not the backslash itself.

In the former case, later meta-characters are left undisturbed and can be used to subvert the sanitization. In the latter case, preceding a meta-character with a backslash leads to the backslash being escaped, but the meta-character appearing un-escaped, which again makes the sanitization ineffective.

Even if the escaped string is not used in a security-critical context, incomplete escaping may still have undesirable effects, such as badly rendered or confusing output.

Recommendation

Use a (well-tested) sanitization library if at all possible. These libraries are much more likely to handle corner cases correctly than a custom implementation.

An even safer alternative is to design the application so that sanitization is not needed, for instance by using prepared statements for SQL queries.

Otherwise, make sure to use a regular expression with the g flag to ensure that all occurrences are replaced, and remember to escape backslashes if applicable.

Example

For example, assume that we want to embed a user-controlled string accountNumber into a SQL query as part of a string literal. To avoid SQL injection, we need to ensure that the string does not contain un-escaped single-quote characters. The following function attempts to ensure this by doubling single quotes, and thereby escaping them:

function escapeQuotes(s) {
  return s.replace("'", "''");
}

As written, this sanitizer is ineffective: if the first argument to replace is a string literal (as in this case), only the first occurrence of that string is replaced.

As mentioned above, the function escapeQuotes should be replaced with a purpose-built sanitization library, such as the npm module sqlstring. Many other sanitization libraries are available from npm and other sources.

If this is not an option, escapeQuotes should be rewritten to use a regular expression with the g ("global") flag instead:

function escapeQuotes(s) {
  return s.replace(/'/g, "''");
}

Note that it is very important to include the global flag: s.replace(/'/, "''") without the global flag is equivalent to the first example above and only replaces the first quote.

References

• OWASP Top 10: A1 Injection.
• npm: sqlstring package.
• Common Weakness Enumeration: CWE-20.
• Common Weakness Enumeration: CWE-80.
• Common Weakness Enumeration: CWE-116.

Incomplete multi-character sanitization

Sanitizing untrusted input is a common technique for preventing injection attacks and other security vulnerabilities. Regular expressions are often used to perform this sanitization. However, when the regular expression matches multiple consecutive characters, replacing it just once can result in the unsafe text re-appearing in the sanitized input.

Attackers can exploit this issue by crafting inputs that, when sanitized with an ineffective regular expression, still contain malicious code or content. This can lead to code execution, data exposure, or other vulnerabilities.

Recommendation

To prevent this issue, it is highly recommended to use a well-tested sanitization library whenever possible. These libraries are more likely to handle corner cases and ensure effective sanitization.

If a library is not an option, you can consider alternative strategies to fix the issue. For example, applying the regular expression replacement repeatedly until no more replacements can be performed, or rewriting the regular expression to match single characters instead of the entire unsafe text.

Example

Consider the following Ruby code that aims to remove all HTML comment start and end tags:

str.gsub(/<!--|--!?>/, "")  

Given the input string "<!<!--- comment --->>", the output will be "<!-- comment -->", which still contains an HTML comment.

One possible fix for this issue is to apply the regular expression replacement repeatedly until no more replacements can be performed. This ensures that the unsafe text does not re-appear in the sanitized input, effectively removing all instances of the targeted pattern:

def remove_html_comments(input)  
  previous = nil  
  while input != previous  
    previous = input  
    input = input.gsub(/<!--|--!?>/, "")  
  end  
  input  
end  

Example

Another example is the following regular expression intended to remove script tags:

str.gsub(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/, "")  

If the input string is "<scrip<script>is removed</script>t>alert(123)</script>", the output will be "<script>alert(123)</script>", which still contains a script tag.

A fix for this issue is to rewrite the regular expression to match single characters ("<" and ">") instead of the entire unsafe text. This simplifies the sanitization process and ensures that all potentially unsafe characters are removed:

def remove_all_html_tags(input)  
  input.gsub(/<|>/, "")  
end  

Another potential fix is to use the popular sanitize gem. It keeps most of the safe HTML tags while removing all unsafe tags and attributes.

require 'sanitize'

def sanitize_html(input)
  Sanitize.fragment(input)
end

Example

Lastly, consider a path sanitizer using the regular expression /\.\.\//:

str.gsub(/\.\.\//, "")  

The regular expression attempts to strip out all occurences of /../ from str. This will not work as expected: for the string /./.././, for example, it will remove the single occurrence of /../ in the middle, but the remainder of the string then becomes /../, which is another instance of the substring we were trying to remove.

A possible fix for this issue is to use the File.sanitize function from the Ruby Facets gem. This library is specifically designed to handle path sanitization, and should handle all corner cases and ensure effective sanitization:

require 'facets'  
  
def sanitize_path(input)  
  File.sanitize(input)  
end  

References

• OWASP Top 10: A1 Injection.
• Stack Overflow: Removing all script tags from HTML with JS regular expression.
• Common Weakness Enumeration: CWE-20.
• Common Weakness Enumeration: CWE-80.
• Common Weakness Enumeration: CWE-116.

Incomplete string escaping or encoding

Sanitizing untrusted input is a common technique for preventing injection attacks such as SQL injection or cross-site scripting. Usually, this is done by escaping meta-characters such as quotes in a domain-specific way so that they are treated as normal characters.

However, directly using the String#sub method to perform escaping is notoriously error-prone. Common mistakes include only replacing the first occurrence of a meta-character, or backslash-escaping various meta-characters but not the backslash itself.

In the former case, later meta-characters are left undisturbed and can be used to subvert the sanitization. In the latter case, preceding a meta-character with a backslash leads to the backslash being escaped, but the meta-character appearing un-escaped, which again makes the sanitization ineffective.

Even if the escaped string is not used in a security-critical context, incomplete escaping may still have undesirable effects, such as badly rendered or confusing output.

Recommendation

Use a (well-tested) sanitization library if at all possible. These libraries are much more likely to handle corner cases correctly than a custom implementation.

An even safer alternative is to design the application so that sanitization is not needed. Otherwise, make sure to use String#gsub rather than String#sub, to ensure that all occurrences are replaced, and remember to escape backslashes if applicable.

Example

As an example, assume that we want to embed a user-controlled string account_number into a SQL query as part of a string literal. To avoid SQL injection, we need to ensure that the string does not contain un-escaped single-quote characters. The following method attempts to ensure this by doubling single quotes, and thereby escaping them:

def escape_quotes(s)
  s.sub "'", "''"
end

As written, this sanitizer is ineffective: String#sub will replace only the first occurrence of that string.

As mentioned above, the method escape_quotes should be replaced with a purpose-built sanitizer, such as ActiveRecord::Base::sanitize_sql in Rails, or by using ORM methods that automatically sanitize parameters.

If this is not an option, escape_quotes should be rewritten to use the String#gsub method instead:

def escape_quotes(s)
  s.gsub "'", "''"
end

References

• OWASP Top 10: A1 Injection.
• Rails: ActiveRecord::Base::sanitize_sql.
• Common Weakness Enumeration: CWE-20.
• Common Weakness Enumeration: CWE-80.
• Common Weakness Enumeration: CWE-116.