[JavaScript] - Incomplete string escaping or encoding

[sridharpratapa]

Description of the issue

File: [javascript/ql/src/Security/CWE-116/IncompleteSanitization.ql]
Description: A string transformer that does not replace or escape all occurrences of a meta-character may be ineffective.

Usage: "WorkflowId": workflowId.replace("}", "")

Issue: This replaces only the first occurrence of "}", but not all the occurrences.

workflowId is a system generated GUID and contains single occurrence of "{" & "}"

Sample Input & Output:
Input: {9ca385f1-88d7-ec11-a7b5-002248283310}
Output: 9ca385f1-88d7-ec11-a7b5-002248283310

In our scenario, we are using replace() function to replace only first occurrence of a character in a system generated GUID (not user input). As per the exception, it is suggested to use '/g' or regular expression to fix all the occurrences of any replacement character. But replacement of all the occurrences is not valid in our scenario.

Is this a valid rule that should be applied to any scenario (like ours)? or applicable only for few scenarios like for sanitizing user inputs or for rendering the data etc.

[tausbn]

Thank you for your question!

As far as I understand it, this rule is only intended for cases where input sanitization takes place. From your description, it does not sound like that's what you're using replace for, and so I would call this a false positive that can be safely ignored.

(cc. @github/codeql-javascript in case I'm mistaken about the nature of this result.)

[erik-krogh]

Is this a valid rule that should be applied to any scenario (like ours)? or applicable only for few scenarios like for sanitizing user inputs or for rendering the data etc.

It is a common bug that happens both when parsing user input or when parsing many other kinds of inputs.
If you look at some sample alerts, then there are plenty of obvious bugs, but also some false positives similar to your example.

I think you should just dismiss the alert you got.

If you got an idea as to how we could remove FPs like yours, then I'm open for ideas.

[ntewari007]

/:/g, '\:' What to replace here! giving Incomplete string escaping or encoding error