Skip to content

integer overflow in session.c:1353 #16290

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
chongwick opened this issue Oct 7, 2024 · 3 comments
Closed

integer overflow in session.c:1353 #16290

chongwick opened this issue Oct 7, 2024 · 3 comments

Comments

@chongwick
Copy link

Description

The following code:

<?php
session_set_cookie_params(PHP_INT_MAX, '/', null, false, true);
session_start();
?>

Resulted in this output:

/home/dan/php-8.3.9/ext/session/session.c:1353:5: runtime error: signed integer overflow: 1728342208 + 9223372036854775807 cannot be represented in type 'long int'

Also, not tested with 8.3.12, yet.

PHP Version

8.3.9

Operating System

No response

@MarcusXavierr
Copy link
Contributor

MarcusXavierr commented Oct 8, 2024

I've tested it on a docker container with PHP 8.3.9 and went fine
image

I've also tested on my machine with PHP 8.2.15 and also didn't had any problem

Did you make any steps besides running this code?

@MarcusXavierr
Copy link
Contributor

MarcusXavierr commented Oct 8, 2024

Maybe the error occurs because the PS(cookie_lifetime) is already the max positive size of long.
image

But why does this overflow doesn't break anything on my machine?

And if the overflow is the problem, and the user can set cookie_lifetime to the max long int value, what are our options? Turning t into unsigned long?
Test the possible overflow before assigning t, and treating the overflow case with the LONG_MAX value from limits.h?

@cmb69
Copy link
Member

cmb69 commented Oct 8, 2024

But why does this overflow doesn't break anything on my machine?

That overflow is undefined behavior; it may not necessarily cause issues, but should be fixed nonetheless. To reliably detect it, you either need to debug manually, or to employ something like UBSan.

@cmb69 cmb69 linked a pull request Oct 8, 2024 that will close this issue
devnexen added a commit to devnexen/php-src that referenced this issue Oct 12, 2024
devnexen added a commit to devnexen/php-src that referenced this issue Oct 12, 2024
devnexen added a commit to devnexen/php-src that referenced this issue Oct 12, 2024
devnexen added a commit to devnexen/php-src that referenced this issue Oct 12, 2024
devnexen added a commit to devnexen/php-src that referenced this issue Oct 13, 2024
devnexen added a commit to devnexen/php-src that referenced this issue Oct 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants