Rule 942360: Small bug in the regex cause FP

[Taiki-San]

Type of Issue

False positive.

Description

The rule trigger a false positive for the following pattern: \W asbla from.
More specifically, patterns similar to , aside from will trigger.

This is likely due to the following construct [\W]\s+as\s*?[\\"'`\w]+\s*?from being used instead [\W]\s+as\s+?[\\"'`\w]+\s*?from.
Specifically \s*? instead of \s+?.

Confirmation

[X] I have removed any personal data (email addresses, IP addresses,
passwords, domain names) from any logs posted.

[dune73]

Sorry for the inconvenience and thank you for reporting.

@franbuehler : You reassembled this rule in your regex archaeology project, I think. Anything comment?

[Taiki-San]

No problem, thanks for the quick response.
Do you want me to open a PR implementing this change?

[dune73]

The 184 open issues does not mean we don't care. It just means we're not enough people in the project. :)

In the light of this, a PR would be most welcome. Can't guarantee immediate merge though, we'll need to take a closer look first.

[Taiki-San]

Didn't meant to imply so :)
Created the PR!

[fgsch]

Changing the \s* to \s+ will break e.g. 1 as' from.

I think a better fix would be to add a word boundary:

[\W]\s+as\b\s*?[\\"'`\w]+\s*?from

This still matches the case above and doesn't match , aside from.

Using pcretest:

"[\d\W]\s+as\s+?[\"'`\w]+\s*?from"
, aside from
No match
1 as' from
No match

"[\d\W]\s+as\b\s*?[\"'`\w]+\s*?from"
, aside from
No match
1 as' from
 0: 1 as' from

What do you think?

[Taiki-San]

Yeah, your solution is better, updated the PR.

[emphazer]

@fgsch good solution with \b instead of +.
okay, i could change the regression test to 6 As" from for example

[fgsch]

@emphazer thanks. should we have a separate PR or include it in #1580?

[emphazer]

@fgsch no problemo, i will make a seperate PR today

[fgsch]

Sorry for the delay. The fix is merged now.