Harden boundary check at history_sql.c

XMLWordPrintable

    • Type: Patch request
    • Resolution: Unresolved
    • Priority: Trivial
    • None
    • Affects Version/s: None
    • Component/s: None
    • None
    • Environment:
      Fedora

      Summary: Potential out-of-bounds read of periods[] in db_read_values_by_count() (src/libs/zbxhistory/history_sql.c)

      Description:

      In db_read_values_by_count() the loop iterating over the periods[] array relies solely on the -1 terminator element to stop:

      const int periods[] =

      {SEC_PER_HOUR, 12 * SEC_PER_HOUR, SEC_PER_DAY, SEC_PER_DAY,                        SEC_PER_WEEK, SEC_PER_MONTH, 0, -1}

      ;
      ...
      while (-1 != periods[step] && 1 < count)
      {
          if (0 > (clock_from = clock_to - periods[step]))
         

      {         clock_from = clock_to;         step = ARRSIZE(periods) - 1;       }

          ...
          clock_to -= periods[step];  
          step++;                           
      }

      When clock_to - periods[step] underflows below 0, step is set to the last index (7), the loop body then does step++, making step == 8. On the next condition check periods[step] reads periods[8], one element past the end of the array — an out-of-bounds read (undefined behaviour). Depending on adjacent stack contents the loop may either terminate by luck or keep advancing step further out of bounds.

      step is never validated against the array size; only the -1 sentinel guards the loop.

      Steps to reproduce: Found by static analysis, not reproducible through normal UI operation. The faulty branch is reached only when end_timestamp passed to db_read_values_by_count() is smaller than the first period (SEC_PER_HOUR), i.e. an out-of-range / corrupted timestamp.

      Result: Read past the end of the local periods[] array (undefined behaviour, potential crash or unbounded loop).

      Expected: The loop must not access periods[] beyond its bounds regardless of the end_timestamp value.

      Suggested fix (attached patch): add an explicit bounds check as the first loop condition:

      while (step < ARRSIZE(periods) && -1 != periods[step] && 1 < count)

      Affected file: src/libs/zbxhistory/history_sql.c, function db_read_values_by_count()

            Assignee:
            Maciej Czernielewski
            Reporter:
            Georgii
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated:

                Estimated:
                Original Estimate - 1h
                1h
                Remaining:
                Remaining Estimate - 1h
                1h
                Logged:
                Time Spent - Not Specified
                Not Specified