[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:

  • enable default "Report unknown triggers" notification (it contains {TRIGGER.NAME} macro which is crucial);
  • create a simple trigger with expression like "{host:item.last()}=1";
  • wait until trigger evaluates to OK;
  • connect to the database directly and update trigger expression by hand like so: 
    zbx_10551=> select * from triggers where description like '%trigger%';
 triggerid | expression | description | url | status | value | priority | lastchange | comments | error | templateid | type | state | flags 
-----------+------------+-------------+-----+--------+-------+----------+------------+----------+-------+------------+------+-------+-------
     13562 | {13172}=1  | trigger     |     |      0 |     0 |        0 |          0 |          |       |            |    0 |     0 |     0
(1 row)

zbx_10551=> update triggers set expression='{13172=1' where triggerid=13562;
UPDATE 1
zbx_10551=> select * from triggers where description like '%trigger%';
 triggerid | expression | description | url | status | value | priority | lastchange | comments | error | templateid | type | state | flags 
-----------+------------+-------------+-----+--------+-------+----------+------------+----------+-------+------------+------+-------+-------
     13562 | {13172=1   | trigger     |     |      0 |     0 |        0 |          0 |          |       |            |    0 |     0 |     0
(1 row)
  • wait for server configuration update, trigger recalculation and escalation start.

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 (ZBXNEXT-744), tr[i]. (1.8 style) was used instead of tr-> (2.0 style).

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.

Generated at Fri Apr 19 06:25:30 EEST 2024 using Jira 9.12.4#9120004-sha1:625303b708afdb767e17cb2838290c41888e9ff0.