Open xcrzx opened 6 hours ago
Pinging @elastic/security-solution (Team: SecuritySolution)
Pinging @elastic/security-detections-response (Team:Detections and Resp)
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)
It looks like the regex is not correct. I started looking at this because (\S+|\s+)
initially looked like some regexes that CodeQL flagged recently as performance issues, but in this case the regex itself doesn't seem slow to execute.
/S+
matches any non-whitespace character, so this regex basically matches anything and I suspect the input strings are being split into tons of tiny chunks as a result. I tried replacing the regex with (\s+)
and it's much faster and the tests still pass, is there some other reason we'd want \S+
included?
Notes on recent regex issues: https://github.com/elastic/security-team/issues/10699
/S+ matches any non-whitespace character, so this regex basically matches anything and I suspect the input strings are being split into tons of tiny chunks as a result.
@dplumlee Yep, that's exactly what happens. The merge
function outputs arrays containing tons of elements, each one being a single character (whitespace or non-whitespace) or an empty string.
Also, the merge
function contains 3 nested loops, so the complexity of it seems to be at least cubic based on the regexp and the sizes of the input strings.
Our goal is to simplify the regexp as much as possible. It should break input strings into lines, not individual characters.
Summary
With certain rule upgrade payloads, the
internal/detection_engine/prebuilt_rules/upgrade/_review
endpoint can block the main Node.js thread for an extended period. Locally, I observed a single rule blocking for over 20 seconds:Steps to Reproduce
It's not entirely clear what specific payload causes this, but I traced the issue to the
note
field in a particular rule. Specifically, the issue appears during merge version calculations here:https://github.com/elastic/kibana/blob/12dc8c1bb84091d7c404b0e29654d56113fc5acb/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/multi_line_string_diff_algorithm.ts#L104-L106
The minimal code needed to reproduce the problem is as follows, but the issue may also occur under other conditions and with different fields.
Impact
It's unclear how often rule upgrades may lead to main thread blocking, but since the
upgrade/_review
endpoint might be called relatively frequently, having even a few rules with updates that trigger the issue could result in Kibana being blocked for several minutes. For this reason, I consider this a critical issue for the Rule Customization release.