]> git.ipfire.org Git - thirdparty/rrdtool-1.x.git/commitdiff
Fix UB when calculating median of all-NaN values
authorDmitry Marakasov <amdmi3@amdmi3.ru>
Wed, 26 Jun 2019 18:14:32 +0000 (21:14 +0300)
committerTobias Oetiker <tobi@oetiker.ch>
Thu, 27 Jun 2019 08:49:00 +0000 (10:49 +0200)
The current code contains undefined behavior where all-NaN values
are passed to median. In that case we end up with final_elements==0
in the following branch:

    else {
        rpnstack->s[++stptr] =
            0.5 * (element_ptr[final_elements / 2] +
                   element_ptr[final_elements / 2 - 1]);
    }

and so we use 0 and -1 as element_ptr array indexes. The
latter is ill-formed and leads to a crash in my case. Move the
check which accounts for the last NaN earlier, so we could
push NaN and finish right away.

src/rrd_rpncalc.c

index d3b6b7f1dd2464ce56789fbe9709d86c741bb70d..787d05fe17b4ec3d17d0402dd721bb40c20fb38c 100644 (file)
@@ -1253,16 +1253,17 @@ short rpn_calc(
                     }
                 }
 
+                /* when goodvals and badvals meet, they might have met on a
+                 * NAN, which wouldn't decrease final_elements. so, check
+                 * that now. */
+                if (isnan(*goodvals))
+                    --final_elements;
+
                 stptr -= elements;
                 if (!final_elements) {
                     /* no non-NAN elements; push NAN */
                     rpnstack->s[++stptr] = DNAN;
                 } else {
-                    /* when goodvals and badvals meet, they might have met on a
-                     * NAN, which wouldn't decrease final_elements. so, check
-                     * that now. */
-                    if (isnan(*goodvals))
-                        --final_elements;
                     /* and finally, take the median of the remaining non-NAN
                      * elements. */
                     qsort(element_ptr, final_elements, sizeof(double),