Skip to content

Commit

Permalink
Fix UB when calculating median of all-NaN values
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
AMDmi3 authored and oetiker committed Jun 27, 2019
1 parent bfe03f3 commit 1d700bf
Showing 1 changed file with 6 additions and 5 deletions.
11 changes: 6 additions & 5 deletions src/rrd_rpncalc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down

0 comments on commit 1d700bf

Please sign in to comment.