[ZBX-10551] Possible crash when processing trigger expression with '{' without matching '}' reported by coverity Created: 2016 Mar 21 Updated: 2017 May 30 Resolved: 2016 Apr 21 |
|
Status: | Closed |
Project: | ZABBIX BUGS AND ISSUES |
Component/s: | Server (S) |
Affects Version/s: | 3.2.0alpha1 |
Fix Version/s: | 3.0.3rc1, 3.2.0alpha1 |
Type: | Incident report | Priority: | Minor |
Reporter: | Glebs Ivanovskis (Inactive) | Assignee: | Unassigned |
Resolution: | Fixed | Votes: | 0 |
Labels: | crash, expressions | ||
Remaining Estimate: | Not Specified | ||
Time Spent: | Not Specified | ||
Original Estimate: | Not Specified |
Description |
Found by Coverity (CID 118926). See the code: for (br = tr->expression, bl = strchr(tr->expression, '{'); NULL != bl; bl = strchr(bl, '{')) { ... if (NULL == (br = strchr(bl, '}'))) /* suppose br == NULL here */ { tr[i].new_error = zbx_strdup(tr[i].new_error, "Invalid trigger expression"); tr[i].new_value = TRIGGER_VALUE_UNKNOWN; THIS_SHOULD_NEVER_HAPPEN; break; } ... } if (NULL == tr->new_error) { zbx_strcpy_alloc(&out, &out_alloc, &out_offset, br); /* dereferencing br == NULL */ ... } From one point of view frontend must prohibit such trigger expressions and THIS_SHOULD_NEVER_HAPPEN, but on the other hand we even try to set a meaningful error message for this situation. Just to crash on the next step. I think this should be fixed. Similar situation in extract_numbers() (CID 118924). |
Comments |
Comment by Glebs Ivanovskis (Inactive) [ 2016 Apr 08 ] |
Possible crash scenario for CID 118924:
Server evaluates incorrect trigger expression to "UNKNOWN" and wants to notify users about that. It then tries to extract_numbers() from incorrect trigger expression in order to substitute "$1" in {TRIGGER.NAME} macro in notification message and crashes in the following place: for (s = str; '\0' != *s; s++) /* find start of number */ { ... if (s != str && '{' == *(s - 1)) { /* skip functions '{65432}' */ s = strchr(s, '}'); /* strchr() returns NULL, which is then incremented and dereferenced */ continue; } ... } Crash log: 3658:20160408:095956.371 Got signal [signal:11(SIGSEGV),reason:1,refaddr:0x1]. Crashing ... 3658:20160408:095956.372 ====== Fatal information: ====== 3658:20160408:095956.372 Program counter: 0x43bdca 3658:20160408:095956.372 === Registers: === 3658:20160408:095956.372 r8 = 0 = 0 = 0 3658:20160408:095956.372 r9 = 8 = 8 = 8 3658:20160408:095956.372 r10 = 7f88a02befe0 = 140224779513824 = 140224779513824 3658:20160408:095956.372 r11 = 206 = 518 = 518 3658:20160408:095956.372 r12 = 31 = 49 = 49 3658:20160408:095956.372 r13 = 7f88a0e03710 = 140224791328528 = 140224791328528 3658:20160408:095956.372 r14 = 20 = 32 = 32 3658:20160408:095956.372 r15 = 0 = 0 = 0 3658:20160408:095956.372 rdi = 7f1331 = 8327985 = 8327985 3658:20160408:095956.372 rsi = 7d = 125 = 125 3658:20160408:095956.372 rbp = 0 = 0 = 0 3658:20160408:095956.372 rbx = 0 = 0 = 0 3658:20160408:095956.372 rdx = 0 = 0 = 0 3658:20160408:095956.372 rax = 0 = 0 = 0 3658:20160408:095956.372 rcx = 7f889fffde87 = 140224776625799 = 140224776625799 3658:20160408:095956.372 rsp = 7ffd1f1a8290 = 140725125284496 = 140725125284496 3658:20160408:095956.372 rip = 43bdca = 4439498 = 4439498 3658:20160408:095956.372 efl = 10293 = 66195 = 66195 3658:20160408:095956.372 csgsfs = 33 = 51 = 51 3658:20160408:095956.372 err = 4 = 4 = 4 3658:20160408:095956.372 trapno = e = 14 = 14 3658:20160408:095956.372 oldmask = 0 = 0 = 0 3658:20160408:095956.372 cr2 = 1 = 1 = 1 3658:20160408:095956.372 === Backtrace: === 3658:20160408:095956.372 13: src/zabbix_server/zabbix_server: escalator #1 [processed 0 escalations in 0.001341 sec, processing escalations](print_fatal_info+0xae) [0x4627be] 3658:20160408:095956.372 12: src/zabbix_server/zabbix_server: escalator #1 [processed 0 escalations in 0.001341 sec, processing escalations]() [0x462a97] 3658:20160408:095956.372 11: /lib/x86_64-linux-gnu/libc.so.6(+0x36d40) [0x7f889ff38d40] 3658:20160408:095956.372 10: src/zabbix_server/zabbix_server: escalator #1 [processed 0 escalations in 0.001341 sec, processing escalations](substitute_simple_macros+0xeda) [0x43bdca] 3658:20160408:095956.372 9: src/zabbix_server/zabbix_server: escalator #1 [processed 0 escalations in 0.001341 sec, processing escalations](substitute_simple_macros+0x3810) [0x43e700] 3658:20160408:095956.372 8: src/zabbix_server/zabbix_server: escalator #1 [processed 0 escalations in 0.001341 sec, processing escalations]() [0x42a5f2] 3658:20160408:095956.372 7: src/zabbix_server/zabbix_server: escalator #1 [processed 0 escalations in 0.001341 sec, processing escalations]() [0x42b38f] 3658:20160408:095956.372 6: src/zabbix_server/zabbix_server: escalator #1 [processed 0 escalations in 0.001341 sec, processing escalations](escalator_thread+0x137) [0x42b617] 3658:20160408:095956.372 5: src/zabbix_server/zabbix_server: escalator #1 [processed 0 escalations in 0.001341 sec, processing escalations](zbx_thread_start+0x45) [0x463345] 3658:20160408:095956.372 4: src/zabbix_server/zabbix_server: escalator #1 [processed 0 escalations in 0.001341 sec, processing escalations](MAIN_ZABBIX_ENTRY+0x5c4) [0x4162b4] 3658:20160408:095956.372 3: src/zabbix_server/zabbix_server: escalator #1 [processed 0 escalations in 0.001341 sec, processing escalations](daemon_start+0x1ad) [0x4620bd] 3658:20160408:095956.372 2: src/zabbix_server/zabbix_server: escalator #1 [processed 0 escalations in 0.001341 sec, processing escalations](main+0x414) [0x4112f4] 3658:20160408:095956.372 1: /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5) [0x7f889ff23ec5] 3658:20160408:095956.372 0: src/zabbix_server/zabbix_server: escalator #1 [processed 0 escalations in 0.001341 sec, processing escalations]() [0x41151f] |
Comment by Glebs Ivanovskis (Inactive) [ 2016 Apr 08 ] |
At the moment execution can never reach the code mentioned in CID 118926 even if the database is corrupted and there are invalid trigger expressions. However, code is obviously wrong. It is a result of r26293 merge ( |
Comment by Glebs Ivanovskis (Inactive) [ 2016 Apr 08 ] |
Fix for version 3.0 is available in development branch svn://svn.zabbix.com/branches/dev/ZBX-10551 revision 59361. |
Comment by Sandis Neilands (Inactive) [ 2016 Apr 11 ] |
Successfully tested. |
Comment by Glebs Ivanovskis (Inactive) [ 2016 Apr 20 ] |
Fixed in pre-3.0.3rc1 r59566, pre-3.1.0 (trunk) r59567. |