Fixes #73373 (deflate_add does not verify that output was not truncated)#2172
Fixes #73373 (deflate_add does not verify that output was not truncated)#2172mbonneau wants to merge 2 commits intophp:masterfrom
Conversation
mbonneau
changed the title
| /* more output buffer space needed; realloc and try again */ | ||
| /* adding 64 more bytes solved every issue I have seen */ | ||
| /* the + 1 is for the string terminator added below */ | ||
| out = zend_string_realloc(out, ZSTR_LEN(out) + 64 + 1, 0); |
There was a problem hiding this comment.
Not sure about this +1. I don't know about zlib, so just an observation.
- There wasn't a +1 before. Why is it needed now?
- Performing this +1 inside the loop will add an extra 1 on each iteration.
There was a problem hiding this comment.
I am pretty sure though the compiler will elide 64 + 1 to + 65 ...
There was a problem hiding this comment.
I think the point was that zend_string_realloc accepts a length. The extra byte for the terminator is added implicitly.
There was a problem hiding this comment.
Oh. Makes sense, though it doesn't make any difference. I already pushed it now; doesn't really matter.
| /* more output buffer space needed; realloc and try again */ | ||
| /* adding 64 more bytes solved every issue I have seen */ | ||
| /* the + 1 is for the string terminator added below */ | ||
| out = zend_string_realloc(out, ZSTR_LEN(out) + 64 + 1, 0); |
There was a problem hiding this comment.
Should check for overflow here.
There was a problem hiding this comment.
overflow? Why? When an addition of 64 will overflow, we already will long have had an out of memory before...
|
@bwoebi IIRC you worked on this, can you take a look? |
6 participants
https://bugs.php.net/bug.php?id=73373