Signed integer overflow

[TedLyngmo]

Make signed integer overflow in redisPollMillis less likely

On platforms where long is 32 bit, the multiplication with 1000 causes UndefinedSanitizer to report signed integer overflows:

net.c:283:24: runtime error: signed integer overflow: 19994637 * 1000 cannot be represented in type 'long int'

This change makes sure that the calculation is made using uint_least64_t and also makes sure that poll() isn't called with a value larger than INT_MAX, which a long (and uint_least64_t) may hold.

---

Note

Medium Risk
Touches connection timeout/polling logic used during socket connect; while intended to be behavior-preserving, changes to time calculations and EINTR handling could subtly affect connect timeouts across platforms.

Overview
Reduces the chance of signed integer overflow in connection timing code by switching timeout and monotonic-time calculations from long to int_least64_t (using INT64_C(1000) multipliers) and adding <stdint.h>.

Hardens redisContextWaitReady by clamping the poll() timeout argument to INT_MAX and recalculating remaining time after EINTR, ensuring large configured timeouts don’t pass out-of-range values to poll() while preserving overall deadline behavior.

^{Written by Cursor Bugbot for commit 3e91d33. This will update automatically on new commits. Configure here.}

[jit-ci]

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

[michael-grunder]

Thanks,

Changing c to ctx adds noise to the PR where lines show as changed but it's just variable renaming. I'm not married to the single letter variable name but it'll clutter up any future git blame 😄

Why int_least64_t as opposed to int64_t?

logic looks good at first glance though.

[TedLyngmo]

Changing c to ctx adds noise to the PR where lines show as changed but it's just variable renaming. I'm not married to the single letter variable name but it'll clutter up any future git blame 😄

😄 I'll change back!

Why int_least64_t as opposed to int64_t?

To not lock-in any assumptions about the width since mostly native types are used elsewhere. Also int64_t is optional and int_least64_t is not.

logic looks good at first glance though.

Great, thanks! I'll clean it up in a few hours!

[TedLyngmo]

... but it'll clutter up any future git blame 😄

Changed ctx back to c but I couldn't help myself and made another change to redisContextWaitReady to initialize wfd directly upon creation. I figured that I had changed so much in that function anyway that it may slip through ... and here I am advertising it!

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

[TedLyngmo]

@michael-grunder Another thing I didn't notice before (since I haven't tested on Windows): redisPollMillis on Windows does return (((long long)ft.dwHighDateTime << 32) | ft.dwLowDateTime) / 10; which returns the microseconds passed since the filesystem epoch, so shouldn't that be return (((long long)ft.dwHighDateTime << 32) | ft.dwLowDateTime) / 10000; to get milliseconds? I can add a fix for that to this PR or should that be made into a separate PR?

[michael-grunder]

hich returns the microseconds passed since the filesystem epoch, so shouldn't that be return (((long long)ft.dwHighDateTime << 32) | ft.dwLowDateTime) / 10000; to get milliseconds?

Yes, /10000 is the right unit conversion. Apparently there is also this:

    return (long long)GetTickCount64();

Which supposedly returns the number of milliseconds since boot docs. Seems like pretty much what we want here.

[TedLyngmo]

    return (long long)GetTickCount64();

Seems like pretty much what we want here.

Indeed, that's even cleaner! Do you want me to add that to this PR?