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

Conversation

@fgsch
Copy link
Contributor

@fgsch fgsch commented Sep 19, 2017

That is, rule 932130 should now match foo as well.

That is, rule 932130 should now match `foo` as well.
@lifeforms
Copy link
Contributor

I purposely left the backquote out of this rule because of a lot of false positives in my test data (it seems that many people use it as quotes, but this was also due to Markdown syntax usage) and would recommend including it only in a higher paranoia level, maybe PL3 or PL2?

@fgsch
Copy link
Contributor Author

fgsch commented Sep 19, 2017

Right, didn't think about Markdown 😞
Should I start with PL2 and move it to PL3 if there are many FPs?

@lifeforms
Copy link
Contributor

My gut feeling would say PL3, since it is quite strong (will block any request that contains two or more ` chars which is not that rare) but let's wait for other opinions!

@csanders-git
Copy link
Contributor

if we did ARGS_GET instead of ARGS i feel like this would be a really strong rule

@fgsch
Copy link
Contributor Author

fgsch commented Sep 19, 2017

if we did ARGS_GET instead of ARGS i feel like this would be a really strong rule

I was thinking around those lines - exclude the body but run it everywhere else.
Maybe split this into 2 rules, one that does not inspect the body and is the default PL, and one that inspects the body in PL >= 2?

@lifeforms
Copy link
Contributor

I like the idea of scanning ARGS_GET by default and scanning the other parameters in a higher level.

@fgsch
Copy link
Contributor Author

fgsch commented Sep 21, 2017

OK, I will work on that and update this PR. Thanks for the input.

@lifeforms lifeforms self-assigned this Oct 2, 2017
@dune73
Copy link
Contributor

dune73 commented Oct 16, 2017

Any progress here @fgsch?

@fgsch
Copy link
Contributor Author

fgsch commented Oct 18, 2017

@dune73 I'm afraid not, been busy. I will get back to this next week.

@dune73
Copy link
Contributor

dune73 commented Oct 28, 2017

There is now a conflict in this PR. Likely due to merging older v3.0 commits with 3.1.

@csanders-git csanders-git self-assigned this Nov 6, 2017
@dune73
Copy link
Contributor

dune73 commented Nov 25, 2017

Any progress here @fgsch?

@lifeforms
Copy link
Contributor

We are still ready to accept this, if the PR is unconflicted (or started again) and only scans ARGS_GET in PL1, and POST in PL2 or PL3.

@dune73
Copy link
Contributor

dune73 commented Dec 14, 2017

Thank you for the fix of the conflict @fzipi.

It looks like @fgsch would be overly busy and can't attack the splitting of the rule and introduction of a higher PL sibling. If somebody else wants to take over, please feel free to do so. I'll review happily.

@fgsch
Copy link
Contributor Author

fgsch commented Dec 14, 2017

I do plan to work on this over xmas but if someone has more bandwidth by all means!

@dune73
Copy link
Contributor

dune73 commented Feb 5, 2018

This has been pending for quite some time, @fgsch? Any plans to return to it?

@fgsch
Copy link
Contributor Author

fgsch commented Feb 22, 2018

Yes, keep planning to, sorry. RL is keeping me busy.

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.

5 participants