Skip to content

Fix 2 segfaults in the range() function #1677

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
wants to merge 8 commits into from

Conversation

tpunt
Copy link
Contributor

@tpunt tpunt commented Dec 15, 2015

Both segfaults occur due to accuracy lost when representing 64bit longs as
doubles.

The segfaults can be replicated with:

range(PHP_INT_MIN + 513, PHP_INT_MIN); // Seg fault on line 2236
range(PHP_INT_MAX - 512, PHP_INT_MAX); // Seg fault on line 2249

This fixes Bug #71132

Both segfaults occur due to accuracy lost when representing 64bit longs as
doubles.

The segfaults can be replicated with:
range(PHP_INT_MIN + 513, PHP_INT_MIN); // Seg fault on line 2236
range(PHP_INT_MAX - 512, PHP_INT_MAX); // Seg fault on line 2249
@laruence laruence added the Bug label Dec 16, 2015
@tpunt tpunt closed this Dec 16, 2015
@tpunt tpunt reopened this Dec 16, 2015
@tpunt tpunt force-pushed the fix-segfaults-in-range-function branch from 4bc9886 to 36bc2fc Compare December 16, 2015 18:18
This caused a few tests to fail because a different error message was being
output
|| (high == ZEND_LONG_MIN && low > -2) /* overflow check */
|| __calc_size > HT_MAX_SIZE /* the array size is too big */
) {
php_error_docref(NULL, E_WARNING, "The supplied range exceeds the maximum array size: start=%ld end=%ld", high, low);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An appropriate sprintf format should be used for zend_long, please see UPGRADING INTERNALS.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks.

@tpunt tpunt force-pushed the fix-segfaults-in-range-function branch 3 times, most recently from ed1f449 to 5433534 Compare December 22, 2015 20:19
@tpunt tpunt force-pushed the fix-segfaults-in-range-function branch 3 times, most recently from 73f14ad to 8e4dd2f Compare December 22, 2015 23:35
@tpunt tpunt force-pushed the fix-segfaults-in-range-function branch from 8e4dd2f to 62f379e Compare December 22, 2015 23:41
By using the calculated array size to determine the number of loop iterations,
we no longer need to perform overflow checks in the loops
@jpauli
Copy link
Member

jpauli commented Dec 24, 2015

Dups with #1690 ?

@staabm
Copy link
Contributor

staabm commented Dec 24, 2015

Per PR description these 2 PRs fix at least different bug-ids

@tpunt
Copy link
Contributor Author

tpunt commented Dec 24, 2015

@jpauli Nope, the segfaults are in different places.

@jpauli
Copy link
Member

jpauli commented Dec 24, 2015

Roger

@tpunt tpunt mentioned this pull request Dec 24, 2015
@tpunt
Copy link
Contributor Author

tpunt commented Dec 24, 2015

Superseded by PR #1695.

@tpunt tpunt closed this Dec 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants