Skip to content

Conversation

@igfoo
Copy link
Contributor

@igfoo igfoo commented Sep 15, 2025

This change corrects the spelling of "occurrences" in the Incomplete Multi-Character Sanitization documentation. This fix aims to improve clarity and ensure that the documentation accurately reflects the intended terminology.

Corrects the spelling of "occurrences" in the Incomplete Multi-Character
Sanitization documentation to improve clarity.
@github-actions
Copy link
Contributor

QHelp previews:

javascript/ql/src/Security/CWE-116/IncompleteMultiCharacterSanitization.qhelp

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 occurrences 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

@igfoo igfoo marked this pull request as ready for review September 15, 2025 14:22
@igfoo igfoo requested a review from a team as a code owner September 15, 2025 14:22
Copilot AI review requested due to automatic review settings September 15, 2025 14:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a spelling error in the documentation for Incomplete Multi-Character Sanitization by correcting "occurences" to "occurrences" in the JavaScript security documentation.

Key Changes

  • Corrected spelling of "occurrences" in security documentation

Copy link
Contributor

@Napalys Napalys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch 👍

@igfoo igfoo merged commit 7860857 into main Sep 15, 2025
11 checks passed
@igfoo igfoo deleted the igfoo/fix-typo branch September 15, 2025 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants