[ZBX-10690] Possible buffer overruns in discovery macros substitution Created: 2016 Apr 20  Updated: 2017 May 30  Resolved: 2016 May 11

Status: Closed
Project: ZABBIX BUGS AND ISSUES
Component/s: Server (S)
Affects Version/s: 2.4.8, 3.0.2, 3.2.0alpha1
Fix Version/s: 3.0.3rc1, 3.2.0alpha1

Type: Incident report Priority: Major
Reporter: Andris Zeila Assignee: Unassigned
Resolution: Fixed Votes: 0
Labels: codequality
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

1) In substitute_discovery_macros() function, user macro parsing - the cursor is moved beyond user macro resulting in skipped character and possible buffer overruns. The fix:

Index: src/libs/zbxserver/expression.c
===================================================================
--- src/libs/zbxserver/expression.c     (revision 59548)
+++ src/libs/zbxserver/expression.c     (working copy)
@@ -4290,8 +4290,8 @@
                        zbx_free(context);
 
                        /* move cursor to the end of user macro */
-                       while ('}' != (*data)[r++])
-                               ;
+                       while ('}' != (*data)[r])
+                               r++;
                }
                /* substitute LLD macros, located in the item key parameters in simple macros */
                /* e.g. {Zabbix server:ifAlias[{#SNMPINDEX}].last(0)}                         */

(added in ZBXNEXT-2683)

2) 1) In substitute_discovery_macros() function, numeric lld macro parsing - if the macro value is negative (ie prefixed with -) the value is enclosed in parentheses (). During this process the replace_to variable is reallocated, but the replace_to_alloc is not updated.
(added in ZBX-10429)

 Comments   
Comment by Andris Zeila [ 2016 Apr 21 ]

(1) The zbx_user_macro_quote_context_dyn() function incorrectly trims leading whitespace from context. While currently this does not affect anything, as it is used only for discovery macro substitution, and in this case macro context will always be quoted. Still it should be fixed with something like this:

Index: src/libs/zbxcommon/str.c
===================================================================
--- src/libs/zbxcommon/str.c    (revision 59451)
+++ src/libs/zbxcommon/str.c    (working copy)
@@ -3574,12 +3576,9 @@
 {
        int             len, quotes = 0;
        char            *buffer, *ptr_buffer;
-       const char      *ptr_context;
+       const char      *ptr_context = context;
 
-       for (ptr_context = context; ' ' == *ptr_context; ptr_context++)
-               ;
-
-       if ('"' == *ptr_context)
+       if ('"' == *ptr_context || ' ' == *ptr_context)
                force_quote = 1;
 
        for (; '\0' != *ptr_context; ptr_context++)

asaveljevs RESOLVED in r59891.

wiper CLOSED

Comment by Andris Zeila [ 2016 Apr 21 ]

(2) In substitute_simple_macros() function - if there was no user macro defined, then zabbix will proceed to parse the macro context. In this case the whole macro including context must be skipped.

For example if expression contains a macro {$M:"{HOST.HOST}"} and there is not {$M} macro defined, then zabbix will expand the {HOST.HOST} macro inside M macro context, which is wrong.

asaveljevs RESOLVED in r59891.

wiper The fix will cause to skip user macros even if user macros are not supported for the specified macro type (although it appears that currently only MACRO_TYPE_ALERT does not support user macros).
For example if we define script media with {$M:"{ALERT.SUBJECT}"} parameter, then it will be expanded to {$M:"{ALERT.SUBJECT}"} instead of {$M:"\<message subject>"}

REOPENED

asaveljevs RESOLVED in r59982.

wiper CLOSED

Comment by Aleksandrs Saveljevs [ 2016 May 05 ]

Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-10690 .

Comment by Andris Zeila [ 2016 May 11 ]

Successfully tested

Comment by Aleksandrs Saveljevs [ 2016 May 11 ]

Fixed in pre-3.0.3rc1 r60025. Since there were many conflicts merging into trunk, will merge there separately.

Comment by Aleksandrs Saveljevs [ 2016 May 11 ]

Fixed for trunk in development branch svn://svn.zabbix.com/branches/dev/ZBX-10690-trunk .

wiper successfully tested

Comment by Aleksandrs Saveljevs [ 2016 May 11 ]

Fixed in pre-3.1.0 (trunk) r60042.

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