Horton

Thread

Commits

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

  1. Fix integer-overflow problem in scram_SaltedPassword()

  1. Fix infinite loop from setting scram_iterations to INT_MAX

    Kevin K Biju <kevinkbiju@gmail.com> — 2025-03-23T13:41:10Z

    Hi,

I stumbled upon a problem with the scram_iterations GUC where setting
scram_iterations to INT_MAX and then creating a user causes the command to
hang indefinitely.

postgres=# SET scram_iterations=2147483647;
SET
postgres=# CREATE ROLE maxscram WITH PASSWORD 'forever';
<hangs>

I looked into the relevant code and found the issue. Each SCRAM iteration
after the first is done in a loop with the following condition:

int i;
...
for (i = 2; i <= iterations; i++)
{
    ...
}

For iterations = INT_MAX, the loop will never terminate since the condition
is <= and adding 1 to INT_MAX will lead to i wrapping around to INT_MIN.

I've fixed this by modifying the loop condition to be i < iterations. I've
attached a patch with the fix. I considered adding a test as well, but
since generating a password with a high number of iterations is very
time-consuming, I'm not sure if that would be practical.

I also tried adding this to the current CommitFest, but my account hasn't
passed the cooldown period yet.

Thanks,
Kevin

  2. Re: Fix infinite loop from setting scram_iterations to INT_MAX

    Richard Guo <guofenglinux@gmail.com> — 2025-03-24T00:50:36Z

    On Sun, Mar 23, 2025 at 10:41 PM Kevin K Biju <kevinkbiju@gmail.com> wrote:
> int i;
> ...
> for (i = 2; i <= iterations; i++)
> {
>     ...
> }
>
> For iterations = INT_MAX, the loop will never terminate since the condition is <= and adding 1 to INT_MAX will lead to i wrapping around to INT_MIN.
>
> I've fixed this by modifying the loop condition to be i < iterations. I've attached a patch with the fix. I considered adding a test as well, but since generating a password with a high number of iterations is very time-consuming, I'm not sure if that would be practical.

Nice catch.  The fix looks good to me.  It seems to me that it's fine
to go without a test case, since the fix is quite straightforward.

Thanks
Richard

  3. Re: Fix infinite loop from setting scram_iterations to INT_MAX

    Michael Paquier <michael@paquier.xyz> — 2025-03-24T00:54:50Z

    On Mon, Mar 24, 2025 at 09:50:36AM +0900, Richard Guo wrote:
> Nice catch.  The fix looks good to me.  It seems to me that it's fine
> to go without a test case, since the fix is quite straightforward.

One could argue about using an injection point to force trick the
iteration loop to be cheaper, but I could not really get excited about
the coverage vs the cycles spent for it..
--
Michael

  4. Re: Fix infinite loop from setting scram_iterations to INT_MAX

    Richard Guo <guofenglinux@gmail.com> — 2025-03-24T00:59:25Z

    On Mon, Mar 24, 2025 at 9:54 AM Michael Paquier <michael@paquier.xyz> wrote:
> On Mon, Mar 24, 2025 at 09:50:36AM +0900, Richard Guo wrote:
> > Nice catch.  The fix looks good to me.  It seems to me that it's fine
> > to go without a test case, since the fix is quite straightforward.
>
> One could argue about using an injection point to force trick the
> iteration loop to be cheaper, but I could not really get excited about
> the coverage vs the cycles spent for it..

Exactly.

Thanks
Richard

  5. Re: Fix infinite loop from setting scram_iterations to INT_MAX

    Richard Guo <guofenglinux@gmail.com> — 2025-03-26T06:41:10Z

    On Mon, Mar 24, 2025 at 9:59 AM Richard Guo <guofenglinux@gmail.com> wrote:
> On Mon, Mar 24, 2025 at 9:54 AM Michael Paquier <michael@paquier.xyz> wrote:
> > On Mon, Mar 24, 2025 at 09:50:36AM +0900, Richard Guo wrote:
> > > Nice catch.  The fix looks good to me.  It seems to me that it's fine
> > > to go without a test case, since the fix is quite straightforward.

> > One could argue about using an injection point to force trick the
> > iteration loop to be cheaper, but I could not really get excited about
> > the coverage vs the cycles spent for it..

> Exactly.

I plan to go ahead and push Kevin's fix, barring any objections.

Thanks
Richard

  6. Re: Fix infinite loop from setting scram_iterations to INT_MAX

    Daniel Gustafsson <daniel@yesql.se> — 2025-03-26T14:43:17Z

    > On 26 Mar 2025, at 07:41, Richard Guo <guofenglinux@gmail.com> wrote:

> I plan to go ahead and push Kevin's fix, barring any objections.

Thanks for picking this up, I had missed this thread.

--
Daniel Gustafsson

  7. Re: Fix infinite loop from setting scram_iterations to INT_MAX

    Michael Paquier <michael@paquier.xyz> — 2025-03-26T22:15:19Z

    On Wed, Mar 26, 2025 at 03:41:10PM +0900, Richard Guo wrote:
> I plan to go ahead and push Kevin's fix, barring any objections.

Thanks, Richard.
--
Michael