Incomplete string escaping or encoding¶
ID: rb/incomplete-sanitizationKind: problemSecurity severity: 7.8Severity: warningPrecision: highTags: - correctness - security - external/cwe/cwe-020 - external/cwe/cwe-080 - external/cwe/cwe-116Query suites: - ruby-code-scanning.qls - ruby-security-extended.qls - ruby-security-and-quality.qls
Click to see the query in the CodeQL repository
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 theString#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 useString#gsub rather thanString#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 stringaccount_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:
defescape_quotes(s)s.sub"'","''"end
As written, this sanitizer is ineffective:String#sub will replace only thefirst occurrence of that string.
As mentioned above, the methodescape_quotes should be replaced with a purpose-built sanitizer, such asActiveRecord::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 theString#gsub method instead:
defescape_quotes(s)s.gsub"'","''"end
References¶
OWASP Top 10:A1 Injection.
Common Weakness Enumeration:CWE-20.
Common Weakness Enumeration:CWE-80.
Common Weakness Enumeration:CWE-116.