8000 Fix FP 953120 brotli compression by franbuehler · Pull Request #2101 · coreruleset/coreruleset · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix FP 953120 brotli compression #2101

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Fix FP 953120 brotli compression #2101

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions rules/RESPONSE-953-DATA-LEAKAGES-PHP.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,10 @@ SecRule RESPONSE_BODY "@rx <\?(?!xml)" \
SecRule RESPONSE_BODY "!@rx (?:\x1f\x8b\x08|\b(?:(?:i(?:nterplay|hdr|d3)|m(?:ovi|thd)|r(?:ar!|iff)|(?:ex|jf)if|f(?:lv|ws)|varg|cws)\b|gif)|B(?:%pdf|\.ra)\b|^wOF[F2])" \
"capture,\
t:none,\
setvar:'tx.outbound_anomaly_score_pl1=+%{tx.error_anomaly_score}',\
setvar:'tx.anomaly_score_pl1=+%{tx.error_anomaly_score}'"
chain"
SecRule RESPONSE_HEADERS:Content-Encoding "!@streq br" \
"setvar:'tx.outbound_anomaly_score_pl1=+%{tx.error_anomaly_score}',\
setvar:'tx.anomaly_score_pl1=+%{tx.error_anomaly_score}'"
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be more efficient to reverse the rules? String equlity check should be much less costly than worst case regexp evaluation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that's smart. I had not thought of that.
I will switch that. Thank you!

Copy link
Member
@dune73 dune73 Jun 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, I am not so sure of that. Because the current first rule is really selective. It will fail most of the time. With the reverse order, we run the non-selective rule first and will have to run the 2nd rule almost always on top. So from the perspective of REQUEST_BODY regex inspection not much changed, but an additional rule is running.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but it's a "not" regexp... and the response text could be massive, I suspect? The regexp would be applied to each line in the response body... (I'm guessing here...)

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's think about this: the negation isn't part of the regular expression but of the annotation. This means that the regular expression will be applied until it finds a match or the end of the input is reached, only then can the result be checked and negated (unless the engine does some magic on top of PCRE). So for every line of input the expression engine will have to check every branch of the expression against the entire line to check for a match, by eye it looks like there are about 3 top level branches. Even with optimizations, I think the number of operations will be greater than for the string equality check (there's also some overhead for parsing the expression or looking it up from the cache).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What should I do??

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, in reality it probably won't matter too much. The performance gain in either case would be marginal. You choose the explanation that sounds best to you, I suggest ;).



SecRule TX:EXECUTING_PARANOIA_LEVEL "@lt 2" "id:953013,phase:3,pass,nolog,skipAfter:END-RESPONSE-953-DATA-LEAKAGES-PHP"
Expand Down
0