Skip to content

removeComments() overzealous parse error and possible security bypass #1207

@katef

Description

@katef

This ticket describes two related issues: a possible security bypass, and a design flaw.

The removeComments() transformation is over zealous, in that it does not consider context for potential comments. For example, char *s = "abc /* ... */ xyz"; would be considered a comment inside the string.

This function also professes to strip comments for several languages, but does not have knowledge of which language it is handling. For example, char *s = "abc -- xyz"; would be considered a comment at the --, and will strip to the end of line. This applies to any language - particularly for SQL injections or XSS similar; I'm just using C syntax here as an example of the general case.

This gives a possible security bypass: If rules match patterns after the removeComments() transformation is applied, then content which should be dissallowed could be placed (e.g. after the string in the above example), and would thus be elided for the rule pattern matching.

To strip comments correctly for multiple languages, the function would need to handle strings for those languages, as in the above examples, Special characters within strings are escaped differently for each language, but this transformation is invoked without knowledge of which language is being parsed.

Without knowledge of the language being parsed, I do not see how this function could ever be implemented correctly.

And a documentation issue for the same: The documentation for removeComments() only described stripping C-style comments. This should be updated to reflect what the function actually does.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions