Skip to content

Potential integer underflow in format_converter() when precision < 0 and adjust_precision == true #18758

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
PavlNekrasov opened this issue Jun 4, 2025 · 5 comments

Comments

@PavlNekrasov
Copy link

Description

Hi,

In the following code, possible precision variable value being set to -1 and adjust_precision == true:

php-src/main/snprintf.c

Lines 576 to 586 in 3a14ce1

adjust_precision = true;
fmt++;
if (isdigit((int)*fmt)) {
STR_TO_DEC(fmt, precision);
} else if (*fmt == '*') {
precision = va_arg(ap, int);
fmt++;
if (precision < -1)
precision = -1;
} else
precision = 0;

This can result undefined behavior when precision is later cast to size_t.

if (adjust_precision && (size_t)precision < s_len) {

Found by Linux Verification Center (linuxtesting.org) with SVACE.
Reporter: Pavel Nekrasov ([email protected]).
Organization: Fobos-NT ([email protected]).

PHP Version

PHP 8.3

Operating System

Alt p10

@devnexen
Copy link
Member

devnexen commented Jun 4, 2025

It is a fine observation, but not sure this code path is actually reached/used by any platform, especially since the switch to C99 (if SVACE works more like cppcheck rather than analysis during build, then it is less surprising). And seems, if I m reading correctly, this bug had been acknowledged already by @derickr

@nielsdos
Copy link
Member

nielsdos commented Jun 4, 2025

How exactly is this undefined behaviour?

@PavlNekrasov
Copy link
Author

How exactly is this undefined behaviour?

Sorry, you're right — it's not undefined behavior, but it does cause an underflow when a negative precision is cast to size_t.

@nielsdos
Copy link
Member

nielsdos commented Jun 9, 2025

I think that's the intention.
By setting precision to -1 you enforce that (size_t)precision < s_len will never be true. That means that the string won't be shortened. This seems right.

@PavlNekrasov
Copy link
Author

Yes, you're right! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants