Skip to content
New issue

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

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

Already on GitHub? # to your account

Prevent unsigned overflow in php_handle_swc() #17678

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
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants