Re: New GNU C Library (glibc) security flaw reported on 30 Jan 2024

[Xi Ruoyao <xry111@xry111.site>]

On Thu, 2024-02-01 at 01:51 +0100, Vincent Lefevre wrote:
> On 2024-02-01 02:47:18 +0800, Xi Ruoyao wrote:
> > On Wed, 2024-01-31 at 12:52 -0300, Adhemerval Zanella Netto wrote:
> > 
> > /* snip */
> > 
> > > 
> > > I see this is an manual issue rather than a GNU 'extension' to qsort semantic.
> > > And I think we should fix BZ#31322 by using a transitive comparison instead of
> > > trying to support such cases.
> > 
> > To me the documentation is correct (though arguably in a very subtle
> > way):
> > 
> >    Here is an example of a comparison function which works with an array
> > of numbers of type ‘double’:
> > 
> >      int
> >      compare_doubles (const void *a, const void *b)
> >      {
> >        const double *da = (const double *) a;
> >        const double *db = (const double *) b;
> > 
> >        return (*da > *db) - (*da < *db);
> >      }
> > 
> > It says "numbers."  But NaN literally means, "Not a Number."
> 
> Yes, the point is to sort numbers. But since NaN may occur, the code
> must not yield undefined behavior in such a case. This is the goal
> of NaN: avoid undefined behavior for operations that do not make any
> sense, and be able to detect errors at the end.

When we sort *numbers* NaN cannot be passed to the comparator.  The
standard disallows qsort to randomly construct a value not in the array
and compare it with the provided comparator:

   The implementation shall ensure that the second argument of the
   comparison function (when called from bsearch), or both arguments
   (when called from qsort), are pointers to elements of the array.  The
   first argument when called from bsearch shall equal key.
   
If the Glibc qsort implementation really construct an NaN and then compare
it with something using the comparator, then this qsort implementation
would be completely wrong and we should assign a CVE number for Glibc.  But
if I read the advisory correct this has not happened, at all.

So if the programmer knows for sure he's sorting an array of numbers,
this comparator is perfectly correct.  It's only wrong when there is one
or multiple NaN in the array.

And I doubt if silently producing a NaN is really good for error
detection.  Simply crashing when an invalid operation happens is easier
for debugging, IMO.  And it's possible with "feenableexcept
(FE_INVALID)" (where FP exceptions are supported).

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University