Skip to content
This repository was archived by the owner on May 14, 2020. It is now read-only.

Conversation

@dune73
Copy link
Contributor

@dune73 dune73 commented Dec 16, 2018

This includes a new rule among the XSS rules and a couple of tests to go with it.

@fgsch
Copy link
Contributor

fgsch commented Jan 7, 2019

Reminder: capture might be missing.

@dune73
Copy link
Contributor Author

dune73 commented Jan 8, 2019

You're right @fgsch. Thanks for pointing this out. I'll update the PR.

Copy link
Contributor

@spartantri spartantri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The space in the regex doesn't change the detection rate "@rx ![!+]\[\]"

@spartantri
Copy link
Contributor

jsfuck payloads are huge and not really understandable, I'm ok to have the E part in the auditlog but I'm curious about why adding %{MATCHED_VAR}?

@dune73
Copy link
Contributor Author

dune73 commented Jan 8, 2019

The comments carry the following remark:

ModSecurity always transforms "+" into " " with query strings and the
URLENCODE body processor
...

Did you notice that?

Copy link
Contributor

@spartantri spartantri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)

@spartantri
Copy link
Contributor

The comments carry the following remark:

ModSecurity always transforms "+" into " " with query strings and the
URLENCODE body processor
...

Did you notice that?

my bad sorry :) it may be interesting to have this tested in front of a WP or some other CMS that uses lots of [] on their payloads before releasing 3.2

@fgsch
Copy link
Contributor

fgsch commented Jan 9, 2019

@dune73 you might want to rebase your branch using the latest commit in v3.2/dev so the tests pass.

@dune73
Copy link
Contributor Author

dune73 commented Jan 11, 2019

@fgsch: That's what I planned to do, but did not have time earlier. Now I did and it messed it up. So I'm planning to throw this away and do a new, clean PR.

Here is what I did. What's wrong with this?

$> git checkout jsfuck-support
$> git remote add upstream [email protected]:SpiderLabs/owasp-modsecurity-crs.git
$> git rebase upstream/v3.2/dev
$> git push origin

I see the tests passing now. But anyways, I guess I should have cherrypicked the right commit.

@fgsch
Copy link
Contributor

fgsch commented Jan 11, 2019

@dune73 not sure. I normally do the following (assuming origin points to this repo):

  1. git co v3.2/dev
  2. git pull origin
  3. git co jsfuck-support
  4. git rebase v3.2/dev
  5. git push -f origin

You should be able to fix this PR without closing it, for example:

  1. git branch -M jsfuck-support jsfuck-support-old
  2. git co v3.2/dev
  3. git pull
  4. git co -b jsfuck-support
  5. git cherry-pick <rev for commit in jsfuck-support-old>
  6. git push -f

@dune73
Copy link
Contributor Author

dune73 commented Jan 12, 2019

That worked nicely. Thank you @fgs.

Merging now. At last. :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants