Skip to content

Prevent unsigned overflow in php_handle_swc() #17678

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

Merged
merged 1 commit into from
Feb 10, 2025

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Feb 3, 2025

The multiplication of ZSTR_LEN(bufz) with the factor can easily overflow on LLP64 architectures, causing a smaller buf to be allocated than expected. While there are no security implications, calling uncompress() with the small buffer cannot be successful (Z_BUF_ERROR). We avoid such superfluous calls by bailing out of the loop early in case of an overflow condition.

Note that safe_emalloc() would not help here, since that will not prevent 32bit unsigned overflow on 64bit architectures.

The multiplication of `ZSTR_LEN(bufz)` with the `factor` can easily
overflow on LLP64 architectures, causing a smaller `buf` to be
allocated than expected.  While there are no security implications,
calling `uncompress()` with the small buffer cannot be successful
(`Z_BUF_ERROR`).  We avoid such superfluous calls by bailing out of
the loop early in case of an overflow condition.

Note that `safe_emalloc()` would not help here, since that will not
prevent 32bit unsigned overflow on 64bit architectures.
@cmb69 cmb69 marked this pull request as ready for review February 3, 2025 12:37
@cmb69 cmb69 requested a review from bukka as a code owner February 3, 2025 12:37
Copy link
Member

@devnexen devnexen left a comment

Choose a reason for hiding this comment

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

MSTM

@cmb69 cmb69 merged commit e6c570a into php:master Feb 10, 2025
9 checks passed
@cmb69 cmb69 deleted the cmb/swc-uncompress-overflow branch February 10, 2025 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants