Fixed bug #73373 (deflate_add does not verify that output was not truncated)

diff --git a/NEWS b/NEWS
index 002fec717c3571cf2c8d3533063e2300035857c5..d22471418ee6043f1e593c70b52b8da7b0b99d53 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -42,6 +42,10 @@ PHP                                                                        NEWS
   . Fixed bug #73594 (dns_get_record does not populate $additional out parameter).
     (Bruce Weirdan)
 
+- Zlib:
+  . Fixed bug #73373 (deflate_add does not verify that output was not truncated).
+    (Matt Bonneau)
+
 08 Dec 2016 PHP 7.0.14
 
 - Core:

diff --git a/ext/zlib/tests/deflate_add_buffer_full.phpt b/ext/zlib/tests/deflate_add_buffer_full.phpt
new file mode 100644 (file)
index 0000000..a2b3fc4
--- /dev/null
+++ b/ext/zlib/tests/deflate_add_buffer_full.phpt
@@ -0,0 +1,53 @@
+--TEST--
+Test deflate_add() buffer issue with data that fills deflate buffer while using ZLIB_SYNC_FLUSH on ZLIB_ENCODING_RAW.
+--SKIPIF--
+<?php 
+if (!extension_loaded("zlib")) {
+    print "skip - ZLIB extension not loaded"; 
+}
+?>
+--FILE--
+<?php
+
+/*
+ * When using ZLIB_ENCODING_RAW, the deflated buffer should always end in 00 00 ff ff
+ * Many streaming deflate users rely on this behaviour.
+ * example: websocket permessage-deflate extension
+ * (https://tools.ietf.org/html/draft-ietf-hybi-permessage-compression-28#section-7.2.1)
+ *
+ * Prior to fixing, the output buffer size was not being checked. According to the zlib
+ * manual, deflate must be called again with more buffer space.
+ */
+
+$deflateContext = deflate_init(ZLIB_ENCODING_RAW);
+
+$deflated = deflate_add(
+    $deflateContext,
+    hex2bin("255044462d312e320a25c7ec8fa20a362030206f626a0a3c3c2f4c656e6774682037203020522f46696c746572202f466c6174654465636f64653e3e0a737472"),
+    ZLIB_SYNC_FLUSH
+);
+
+echo bin2hex(substr($deflated, strlen($deflated) - 4)) . "\n";
+
+$deflated = deflate_add(
+    $deflateContext,
+    hex2bin("65616d0a789c7d53c16ed43010bde7c037f85824766a7bc6767c2ca8a00a016a1b2edcb2dbecaed1266937d98afe3d6327363794439437e3f17b6f5e242821e3"),
+    ZLIB_SYNC_FLUSH
+);
+
+echo bin2hex(substr($deflated, strlen($deflated) - 4)) . "\n";
+
+$deflated = deflate_add(
+    $deflateContext,
+    hex2bin("b3be777df5525d3f90384cd58b50a9945fbb5e7c6cb8c89fca8156c688665f2de794504a81f75658a7c1d54a347d7575fb6e17ba617edffcae9c84da3aee6c9e"),
+    ZLIB_SYNC_FLUSH
+);
+
+echo bin2hex(substr($deflated, strlen($deflated) - 4)) . "\n";
+?>
+===DONE===
+--EXPECTF--
+0000ffff
+0000ffff
+0000ffff
+===DONE===

diff --git a/ext/zlib/zlib.c b/ext/zlib/zlib.c
index d9d6be16383cdcc1b2f0101a7e9e07c1d3c6e2ef..bbe1334ca97dbc3f3d7db1f6ed3ada7bd5080f29 100644 (file)
--- a/ext/zlib/zlib.c
+++ b/ext/zlib/zlib.c
@@ -1115,7 +1115,7 @@ PHP_FUNCTION(deflate_add)
 {
        zend_string *out;
        char *in_buf;
-       size_t in_len, out_size;
+       size_t in_len, out_size, buffer_used;
        zval *res;
        z_stream *ctx;
        zend_long flush_type = Z_SYNC_FLUSH;
@@ -1157,6 +1157,7 @@ PHP_FUNCTION(deflate_add)
        out_size = PHP_ZLIB_BUFFER_SIZE_GUESS(ctx->total_in + in_len);
        out_size = (ctx->total_out >= out_size) ? 16 : (out_size - ctx->total_out);
        out_size = (out_size < 16) ? 16 : out_size;
+       out_size += 64;
        out = zend_string_alloc(out_size, 0);
 
        ctx->next_in = (Bytef *) in_buf;
@@ -1164,7 +1165,21 @@ PHP_FUNCTION(deflate_add)
        ctx->avail_in = in_len;
        ctx->avail_out = ZSTR_LEN(out);
 
-       status = deflate(ctx, flush_type);
+       buffer_used = 0;
+
+       do {
+               if (ctx->avail_out == 0) {
+                       /* 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);
+                       ctx->avail_out = 64;
+                       ctx->next_out = (Bytef *) ZSTR_VAL(out) + buffer_used;
+               }
+               status = deflate(ctx, flush_type);
+               buffer_used = ZSTR_LEN(out) - ctx->avail_out;
+       } while (status == Z_OK && ctx->avail_out == 0);
+
        switch (status) {
                case Z_OK:
                        ZSTR_LEN(out) = (char *) ctx->next_out - ZSTR_VAL(out);