Team C
[ZBX-16103] Lower PCRE recursion limit Created: 2019 May 09 Updated: 2024 Apr 10 Resolved: 2019 Jul 22 |
|
| Status: | Closed |
| Project: | ZABBIX BUGS AND ISSUES |
| Component/s: | Proxy (P), Server (S) |
| Affects Version/s: | 4.0.7, 4.2.1 |
| Fix Version/s: | 4.0.11rc1, 4.2.5rc1, 4.4.0alpha1, 4.4 (plan) |
| Type: | Defect (Security) | Priority: | Critical |
| Reporter: | Glebs Ivanovskis | Assignee: | Aleksejs Sestakovs |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | limits, pcre, preprocessing, regexps, security | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Team: | Team C |
| Sprint: | Sprint 53 (Jun 2019), Sprint 54 (Jul 2019) |
| Story Points: | 0.25 |
| Description |
|
Quoting myself:
For the reference, default value for PHP is 100000 (10 times less). man pcrestack provides some useful insights, particularly:
This can be considered a security flaw, because you don't need tremendous permissions to configure a preprocessing rule with sub-optimal regexp and send a suitable value via zabbix_sender to bring Zabbix server down due to stack overflow (see |
| Comments |
| Comment by Vladislavs Sokurenko [ 2019 May 09 ] |
|
Does lowering this value help with |
| Comment by Glebs Ivanovskis [ 2019 May 10 ] |
|
It does. Stack is limited to 8 MB on my machine: $ ulimit -a | grep stack stack size (kbytes, -s) 8192 With recursion limit lowered to 16000 Zabbix server does not crash when performing preprocessing: 17280:20190510:001748.318 item "Testing:vfs.file.contents[/tmp/payload_raw.txt]" became not supported: Preprocessing failed for: {"clusterStatus":{"rollupRequestedDeploymentState":"Unknown","href":"/api/0.5/status","nodes":[{"...
1. Failed: cannot perform regular expression "node2(.|\n)*?rollupStatus": "(.*)"\n" match for value of type "string": pattern does not match
Recommendation in PCRE docs is spot on! When limit is raised even to 17000 Zabbix starts to crash again. |
| Comment by Vladislavs Sokurenko [ 2019 May 10 ] |
|
Yes, but 16000 is lower than default in PHP which is 100000, so it would crash anyway. Which value is suggested as a new limit, is it 16000 ? Is it possible that someone might need more ? Perhaps it is worth also to document current recursion limit ? |
| Comment by Glebs Ivanovskis [ 2019 May 30 ] |
|
Sorry for a delayed response. PHP default does not have to be precise, because it is just a default value for a configurable limit. Making PCRE limits configurable in Zabbix would be perfect, but it would be a feature request which could only be delivered in a major version since one cannot introduce a new configuration parameter in a minor release. "500 bytes per recursion" heuristic seems to work pretty well and application can find out its stack size limit using getrlimit(RLIMIT_STACK, ...), I would suggest to go this way. Hardcoded limit is, no doubt, the simplest solution. If Zabbix decides to go this way, I'd say that lower limit is better. Zabbix exposes regular expressions to end users (in contrast to PHP, where it is typically used by developers) and, unfortunately, least experienced regexp users are the most "lucky" to come up with runaway patterns. Taking into account that suboptimal regexp can cause major performance problems for Zabbix server as a whole, I think it will be nice to detect these CPU hogs as soon as possible. If someone needs more recursion – there is always (or at least in preprocessing) an option to use two chained regular expressions instead of one. |
| Comment by Aleksejs Sestakovs [ 2019 Jul 15 ] |
|
Available in versions:
|