Horton

Re: glibc qsort() vulnerability

Mats Kindahl <mats@timescale.com>

View thread
From: Mats Kindahl <mats@timescale.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Andres Freund <andres@anarazel.de>, Thomas Munro <thomas.munro@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>, pgsql-hackers@lists.postgresql.org
Date: 2024-02-11T14:44:42Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Use new overflow-safe integer comparison functions.

  2. Introduce overflow-safe integer comparison functions.

  3. Replace calls to pg_qsort() with the qsort() macro.

  4. Switch over to using our own qsort() all the time, as has been proposed

On Sat, Feb 10, 2024 at 9:53 PM Nathan Bossart <nathandbossart@gmail.com>
wrote:

> On Sat, Feb 10, 2024 at 08:59:06AM +0100, Mats Kindahl wrote:
> > Split the code into two patches: one that just adds the functions
> > (including the new pg_cmp_size()) to common/int.h and one that starts
> using
> > them. I picked the name "pg_cmp_size" rather than "pg_cmp_size_t" since
> > "_t" is usually used as a suffix for types.
> >
> > I added a comment to the (a > b) - (a < b) return and have also added
> casts
> > to (int32) for the int16 and uint16 functions (we need a signed int for
> > uin16 since we need to be able to get a negative number).
> >
> > Changed the type of two instances that had an implicit cast from size_t
> to
> > int and used the new pg_,cmp_size() function.
> >
> > Also fixed the missed replacements in the "contrib" directory.
>
> Thanks for the new patches.  I think the comparison in resowner.c is
> backwards,


Thanks for catching that.



> and I think we should expand on some of the commentary in int.h.
> For example, the comment at the top of int.h seems very tailored to the
> existing functions and should probably be adjusted.


I rewrote the beginning to the following, does that look good?

 * int.h
 *  Routines to perform signed and unsigned integer arithmetics, including
 *  comparisons, in an overflow-safe way.



> And the "comparison
> routines for integers" comment might benefit from some additional details
> about the purpose and guarantees of the new functions.
>

I expanded that into the following. WDYT?

/*------------------------------------------------------------------------
 * Comparison routines for integers.
 *
 * These routines are used to implement comparison functions for, e.g.,
 * qsort(). They are designed to be efficient and not risk overflows in
 * internal computations that could cause strange results, such as INT_MIN >
 * INT_MAX if you just return "lhs - rhs".
 *------------------------------------------------------------------------


Best wishes,
Mats Kindahl


> --
> Nathan Bossart
> Amazon Web Services: https://aws.amazon.com
>