Python: Add Regular Expression Injection query

[jorgectf]

Any suggestions/tricks to improve the query or workarounds (to learn), no matter how minimal they are, are massively appreciated.

[erik-krogh]

Just a small comment on naming.

There are currently ReDoS queries implemented for C# and JavaScript.
Both of these look for expensive regular expressions running on user input.

Your query looks for regular expressions being constructed from user input.
There is a similar query implemented for JavaScript, which is called Regular expression injection, and I think that name better describes what your query does.
And it would make the naming consistent across languages.

[jorgectf]

Thanks for the detail @erik-krogh! I've updated everything according to Regular Expression Injection instead of Regular Expression Denial of Service.

[yoff]

Regarding the structure we now try to follow:

• RegexInjectionSink might be added to Concepts.qll (and called something like RegexExecution).
• EscapeSanitizer might be moved to Concepts.qll (and called something like RegexEscape)
• The configuration should be moved to its own file (see e.g. StackTraceExposure.ql and StackTraceExposure.qll)

[yoff]

If the sanitiser is moved to concepts, it might have to be made a bit more precise, but I am not sure. Is there an awful lot of ways to escape?

[jorgectf]

If the sanitiser is moved to concepts, it might have to be made a bit more precise, but I am not sure. Is there an awful lot of ways to escape?

As far as I know, re.escape is the only already-built function that escapes the regular expression, but the actual Sanitizer may also work for custom-made escape functions (I guess it'd very strange but could happen). Should it be reduced only to re.escape?

[jorgectf]

RegexInjectionSink might be added to Concepts.qll (and called something like RegexExecution).

Shall I include DirectRegex and CompiledRegex into Concepts.qll too?

[yoff]

As far as I know, re.escape is the only already-built function that escapes the regular expression, but the actual Sanitizer may also work for custom-made escape functions (I guess it'd very strange but could happen). Should it be reduced only to re.escape?

In that case, we should probably limit it for now. We can then expand the concept if we see false positives due to custom escape mechanisms.

[yoff]

RegexInjectionSink might be added to Concepts.qll (and called something like RegexExecution).

Shall I include DirectRegex and CompiledRegex into Concepts.qll too?

We would actually have those predicates that populate the concepts scattered around in the models of the libraries where the concepts occur. So in this case, you identify some regex execution in the re module in the standard library. So it should go into a module named re inside Stdlib.qll. We can then later add other regex executions e.g. from the regex library in Regex.qll next to Stdlib.qll.

[yoff]

As mentioned we will have to put it all into experimental. I will add the required files and folders to main, then you can just pull and move things around.
We also still need qldocs on public methods, but that is a smaller thing.

[yoff]

My apologies, I did not explain the idea behind getRegexNode clearly. I thought I had described how it would change the definition of sinks, but clearly those thoughts did not make it out of my mind. I have given the relevant modifications; this should also make the query compile again :-)

[jorgectf]

Absolutely no problem @yoff! I made some tests yesterday and had to unset the override in RegexExecution's Range since DataFlow's Node has no Range class, but didn't manage to understand what to do with regexNode. Thank you so much for your awesome explanations! :D

[yoff]

#5493 has now been merged, so you have somewhere safe to put all the files :-)

[yoff]

Final consistency polish :-)

[jorgectf]

Final consistency polish :-)

Thanks 👍

[yoff]

LGTM

[yoff]

Just needed to make the auto format check happy..

[jorgectf]

Just needed to make the auto format check happy..

😆

[jorgectf]

🎉