ZABBIX BUGS AND ISSUES
  1. ZABBIX BUGS AND ISSUES
  2. ZBX-10690

Possible buffer overruns in discovery macros substitution

    Details

      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)

        Activity

        Hide
        Andris Zeila added a comment - - edited

        (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++)
        

        Aleksandrs Saveljevs RESOLVED in r59891.

        Andris Zeila CLOSED

        Show
        Andris Zeila added a comment - - edited (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++) Aleksandrs Saveljevs RESOLVED in r59891. Andris Zeila CLOSED
        Hide
        Andris Zeila added a comment - - edited

        (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.

        Aleksandrs Saveljevs RESOLVED in r59891.

        Andris Zeila 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

        Aleksandrs Saveljevs RESOLVED in r59982.

        Andris Zeila CLOSED

        Show
        Andris Zeila added a comment - - edited (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. Aleksandrs Saveljevs RESOLVED in r59891. Andris Zeila 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 Aleksandrs Saveljevs RESOLVED in r59982. Andris Zeila CLOSED
        Hide
        Aleksandrs Saveljevs added a comment -

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

        Show
        Aleksandrs Saveljevs added a comment - Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-10690 .
        Hide
        Andris Zeila added a comment -

        Successfully tested

        Show
        Andris Zeila added a comment - Successfully tested
        Hide
        Aleksandrs Saveljevs added a comment -

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

        Show
        Aleksandrs Saveljevs added a comment - Fixed in pre-3.0.3rc1 r60025. Since there were many conflicts merging into trunk, will merge there separately.
        Hide
        Aleksandrs Saveljevs added a comment - - edited

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

        Andris Zeila successfully tested

        Show
        Aleksandrs Saveljevs added a comment - - edited Fixed for trunk in development branch svn://svn.zabbix.com/branches/dev/ZBX-10690-trunk . Andris Zeila successfully tested
        Hide
        Aleksandrs Saveljevs added a comment -

        Fixed in pre-3.1.0 (trunk) r60042.

        Show
        Aleksandrs Saveljevs added a comment - Fixed in pre-3.1.0 (trunk) r60042.

          People

          • Assignee:
            Unassigned
            Reporter:
            Andris Zeila
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: