Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positive for "wordContainingFile(" in _sanitize_naughty_javascript #99

Closed
Fahl-Design opened this issue Mar 3, 2022 · 4 comments · Fixed by #101
Closed

False positive for "wordContainingFile(" in _sanitize_naughty_javascript #99

Fahl-Design opened this issue Mar 3, 2022 · 4 comments · Fixed by #101

Comments

@Fahl-Design
Copy link
Contributor

Hi, I think i stumbled on a false positive in _sanitize_naughty_javascript

What is this feature about (expected vs actual behaviour)?

  • do no detect XSS

How can I reproduce it?

$str = '<p>Montageprofile(n)</p>';

$antiXss = new \voku\helper\AntiXSS(); 
$antiXss->xss_clean($str);

assert(false === $antiXss->isXssFound());

Does it take minutes, hours or days to fix?

  • minutes, maybe

Any additional information?

https://github.com/voku/anti-xss/blob/master/src/voku/helper/AntiXSS.php#L1680:L1721

  • regex is missing a space before the keyword pattern, blablafile(n) is no valid js
@voku
Copy link
Owner

voku commented Mar 3, 2022

Good catch, can you create a pull request? 👍

@Fahl-Design
Copy link
Contributor Author

Yes for sure :) I will push one later today

Fahl-Design added a commit to Fahl-Design/anti-xss that referenced this issue Mar 4, 2022
…negative look behind

- add test cases
- change js test result (no longer false positive)

Signed-off-by: Benjamin Fahl <git@fahl-design.de>
Fahl-Design added a commit to Fahl-Design/anti-xss that referenced this issue Mar 4, 2022
… false positive cases

Signed-off-by: Benjamin Fahl <git@fahl-design.de>
Fahl-Design added a commit to Fahl-Design/anti-xss that referenced this issue Mar 4, 2022
Signed-off-by: Benjamin Fahl <git@fahl-design.de>
@Fahl-Design
Copy link
Contributor Author

I fixed this one and found an other, _do_never_allowed_afterwards is a bit "aggressive" when it comes to "onEnd" events, I will give it a try tomorrow,
the PR is able to merge anyway

Fahl-Design added a commit to Fahl-Design/anti-xss that referenced this issue Mar 4, 2022
Signed-off-by: Benjamin Fahl <git@fahl-design.de>
@voku voku closed this as completed in #101 Mar 6, 2022
voku added a commit that referenced this issue Mar 6, 2022
…_check

Resolve #99 - Optimize "_sanitize_naughty_javascript"
@mike-healy
Copy link

I think this issue is similar to a problem I'm having with text like system (… being considered dangerous.
For example the string the operating system (e.g. Windows 10, Mac OS X etc.) you are using is considered to contain XSS.

Would a similar change for system work?
@voku , @Fahl-Design

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants