Horton

Thread

Commits

  1. Fix int32 overflow in ltree_compare()

  1. Fw:Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows int32 on deep ltree comparisons, returning the wrong sign

    王跃林 <violin0613@tju.edu.cn> — 2026-06-13T03:50:25Z

    







-------- 转发邮件信息 --------
发件人：Noah Misch <noah@leadboat.com>
发送日期：2026-06-13 08:31:47
收件人："violin0613@tju.edu.cn" <violin0613@tju.edu.cn>
抄送人：security@postgresql.org
主题：Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows int32 on deep ltree comparisons, returning the wrong sign
On Mon, May 25, 2026 at 10:58:05PM +0800, violin0613@tju.edu.cn wrote:
> PoC
> 
> File vuln_001.sql
> 
>  CREATE EXTENSION IF NOT EXISTS ltree;
>  â
>  WITH s AS (SELECT 'a'::ltree AS v),
>       l AS (SELECT (repeat('a.', 19999) || 'a')::ltree AS v)
>  SELECT (l.v > s.v) AS long_gt_short_expected_true,
>         (l.v < s.v) AS long_lt_short_expected_false
>  FROM s, l;
> 
> Process
> 
>  psql -h /tmp -p 36265 -U postgres -f vuln_001.sql
> 
> Results
> 
>   long_gt_short_expected_true | long_lt_short_expected_false
>  -----------------------------+------------------------------
>   f                           | t
> 
>    Both columns are inverted. long > short returned false, long < short
>    returned true.

This is certainly a bug, but it's not a vuln.  Please report the bug to
pgsql-bugs@postgresql.org.

  2. Re: Fw:Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows int32 on deep ltree comparisons, returning the wrong sign

    Ayush Tiwari <ayushtiwari.slg01@gmail.com> — 2026-06-13T06:12:44Z

    Hi,

On Sat, 13 Jun 2026 at 09:20, 王跃林 <violin0613@tju.edu.cn> wrote:

> 主题：Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows int32 on deep ltree comparisons, returning the wrong sign
> On Mon, May 25, 2026 at 10:58:05PM +0800, violin0613@tju.edu.cn wrote:
> > PoC
> >
> > File vuln_001.sql
> >
> >  CREATE EXTENSION IF NOT EXISTS ltree;
> >  â
> >  WITH s AS (SELECT 'a'::ltree AS v),
> >       l AS (SELECT (repeat('a.', 19999) || 'a')::ltree AS v)
> >  SELECT (l.v > s.v) AS long_gt_short_expected_true,
> >         (l.v < s.v) AS long_lt_short_expected_false
> >  FROM s, l;
> >
> > Process
> >
> >  psql -h /tmp -p 36265 -U postgres -f vuln_001.sql
> >
> > Results
> >
> >   long_gt_short_expected_true | long_lt_short_expected_false
> >  -----------------------------+------------------------------
> >   f                           | t
> >
> >    Both columns are inverted. long > short returned false, long < short
> >    returned true.
>
>
This looks like a classic case of integer overflow that's
happening in ltree_compare function in ltree_op.c.

return (al->len - bl->len) * 10 * (an + 1);
return res * 10 * (an + 1);
return (a->numlevel - b->numlevel) * 10 * (an + 1);

I think the calculation should be done as int64, something of this sort:

    int64 v = (int64) (al->len - bl->len) * 10 * (an + 1);
    if (v > PG_INT32_MAX) return PG_INT32_MAX;
    if (v < PG_INT32_MIN) return PG_INT32_MIN;
    return (int) v;

And needed to adjust the ltree_penalty function too.

Attached is a draft patch for this, I guess we can add a helper
function too for the above conversion.

Thoughts?

Regards,
Ayush

  3. Re: Fw:Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows int32 on deep ltree comparisons, returning the wrong sign

    Heikki Linnakangas <hlinnaka@iki.fi> — 2026-06-15T15:08:05Z

    On 13/06/2026 09:12, Ayush Tiwari wrote:
> This looks like a classic case of integer overflow that's
> happening in ltree_compare function in ltree_op.c.
> 
> return (al->len - bl->len) * 10 * (an + 1);
> return res * 10 * (an + 1);
> return (a->numlevel - b->numlevel) * 10 * (an + 1);
> 
> I think the calculation should be done as int64, something of this sort:
> 
>      int64 v = (int64) (al->len - bl->len) * 10 * (an + 1);
>      if (v > PG_INT32_MAX) return PG_INT32_MAX;
>      if (v < PG_INT32_MIN) return PG_INT32_MIN;
>      return (int) v;
> 
> And needed to adjust the ltree_penalty function too.
> 
> Attached is a draft patch for this, I guess we can add a helper
> function too for the above conversion.

Yeah, that works. However, I note that the multiplication is only really 
needed by the ltree_penalty() caller. All the other callers just check 
if the return value is less than, equal, or greater than zero. It feels 
a little silly to do all that work of multiplication and clamping for 
those callers. And for ltree_penalty(), the caller actually converts the 
return value to a float, so clamping it to int32 range feels a little 
silly for that too. So I propose the attached, which splits the 
ltree_compare() function into two variants: one for ltree_penalty() that 
returns a float, and one for others that don't care about the 
"magnitude". It duplicates a little code, but I think it's easier to 
reason about. What do you think?

- Heikki

  4. Re: Fw:Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows int32 on deep ltree comparisons, returning the wrong sign

    Ayush Tiwari <ayushtiwari.slg01@gmail.com> — 2026-06-15T15:24:56Z

    Hi,

On Mon, 15 Jun 2026 at 20:38, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

> On 13/06/2026 09:12, Ayush Tiwari wrote:
> > This looks like a classic case of integer overflow that's
> > happening in ltree_compare function in ltree_op.c.
> >
> > return (al->len - bl->len) * 10 * (an + 1);
> > return res * 10 * (an + 1);
> > return (a->numlevel - b->numlevel) * 10 * (an + 1);
> >
> > I think the calculation should be done as int64, something of this sort:
>
> Yeah, that works. However, I note that the multiplication is only really
> needed by the ltree_penalty() caller. All the other callers just check
> if the return value is less than, equal, or greater than zero. It feels
> a little silly to do all that work of multiplication and clamping for
> those callers. And for ltree_penalty(), the caller actually converts the
> return value to a float, so clamping it to int32 range feels a little
> silly for that too. So I propose the attached, which splits the
> ltree_compare() function into two variants: one for ltree_penalty() that
> returns a float, and one for others that don't care about the
> "magnitude". It duplicates a little code, but I think it's easier to
> reason about. What do you think?
>

I had thought initially of using the method you have added,
(not with float return-type though, I thought of planning to create
a duplicate function with int64 type just for ltree_penalty(), but float
type
is better) noting the same thing that rest callers just care about
comparison with 0.

But thought backpatching it would be harder, hence I used the int64
method. However, your patch looks much better than the ugly
behaviour and I agree it did not make sense to do those
multiplications at all other comparison functions.

Regards,
Ayush

  5. Re: Fw:Re: Fw: ltree_compare in contrib/ltree/ltree_op.c overflows int32 on deep ltree comparisons, returning the wrong sign

    Heikki Linnakangas <hlinnaka@iki.fi> — 2026-06-16T06:37:42Z

    On 15/06/2026 18:24, Ayush Tiwari wrote:
> On Mon, 15 Jun 2026 at 20:38, Heikki Linnakangas <hlinnaka@iki.fi 
> <mailto:hlinnaka@iki.fi>> wrote:
> 
>     On 13/06/2026 09:12, Ayush Tiwari wrote:
>      > This looks like a classic case of integer overflow that's
>      > happening in ltree_compare function in ltree_op.c.
>      >
>      > return (al->len - bl->len) * 10 * (an + 1);
>      > return res * 10 * (an + 1);
>      > return (a->numlevel - b->numlevel) * 10 * (an + 1);
>      >
>      > I think the calculation should be done as int64, something of
>     this sort:
> 
>     Yeah, that works. However, I note that the multiplication is only
>     really
>     needed by the ltree_penalty() caller. All the other callers just check
>     if the return value is less than, equal, or greater than zero. It feels
>     a little silly to do all that work of multiplication and clamping for
>     those callers. And for ltree_penalty(), the caller actually converts
>     the
>     return value to a float, so clamping it to int32 range feels a little
>     silly for that too. So I propose the attached, which splits the
>     ltree_compare() function into two variants: one for ltree_penalty()
>     that
>     returns a float, and one for others that don't care about the
>     "magnitude". It duplicates a little code, but I think it's easier to
>     reason about. What do you think?
> 
> 
> I had thought initially of using the method you have added,
> (not with float return-type though, I thought of planning to create
> a duplicate function with int64 type just for ltree_penalty(), but float 
> type
> is better) noting the same thing that rest callers just care about
> comparison with 0.
> 
> But thought backpatching it would be harder, hence I used the int64
> method. However, your patch looks much better than the ugly
> behaviour and I agree it did not make sense to do those
> multiplications at all other comparison functions.

Great, committed. Thanks!

- Heikki