Propagate errors from stream read/write operations

ext/bz2/bug71263.phpt

@@ -0,0 +1,53 @@
+--TEST--
+Bug #71263: fread() does not detects decoding errors from filter bzip2.decompress
+--FILE--
+<?php
+
+// Should notices be generated?
+
+function test($case) {
+	$plain = "The quick brown fox jumps over the lazy dog.";
+	$fn = "bug71263.bz2";
+	$compressed = (string) bzcompress($plain);
+	echo "Compressed len = ", strlen($compressed), "\n";
+
+    if ($case == 1) {
+        // Set a random byte in the middle of the compressed data
+        // --> php_bz2_decompress_filter() detects fatal error
+        // --> fread() displays empty string then garbage, no errors detected:
+        $compressed[strlen($compressed) - 15] = 'X';
+    } else if ($case == 2) {
+        // Truncate the compressed data
+        // --> php_bz2_decompress_filter() does not detect errors,
+        // --> fread() displays the empty string:
+    	$compressed = substr($compressed, 0, strlen($compressed) - 20);
+    } else {
+        // Corrupted final CRC
+        // --> php_bz2_decompress_filter() detects fatal error
+        // --> fread() displays an empty string, then the correct plain text, no error detected:
+    	$compressed[strlen($compressed)-2] = 'X';
+    }
+
+	file_put_contents($fn, $compressed);
+
+	$r = fopen($fn, "r");
+	stream_filter_append($r, 'bzip2.decompress', STREAM_FILTER_READ);
+    while (!feof($r)) {
+        $s = fread($r, 100);
+        echo "read: "; var_dump($s);
+    }
+	fclose($r);
+	unlink($fn);
+}
+
+test(1);
+test(2);
+test(3);
+?>
+--EXPECT--
+Compressed len = 81
+read: bool(false)
+Compressed len = 81
+read: string(0) ""
+Compressed len = 81
+read: bool(false)

ext/bz2/bz2.c

@@ -133,7 +133,7 @@ struct php_bz2_stream_data_t {
 
 /* {{{ BZip2 stream implementation */
 
-static size_t php_bz2iop_read(php_stream *stream, char *buf, size_t count)
+static ssize_t php_bz2iop_read(php_stream *stream, char *buf, size_t count)
 {
 	struct php_bz2_stream_data_t *self = (struct php_bz2_stream_data_t *)stream->abstract;
 	size_t ret = 0;
@@ -149,6 +149,9 @@ static size_t php_bz2iop_read(php_stream *stream, char *buf, size_t count)
 			/* it is not safe to keep reading after an error, see #72613 */
 			stream->eof = 1;
 			if (just_read < 0) {
+				if (ret) {
+					return ret;
+				}
 				return -1;
 			}
 			break;
@@ -160,20 +163,24 @@ static size_t php_bz2iop_read(php_stream *stream, char *buf, size_t count)
 	return ret;
 }
 
-static size_t php_bz2iop_write(php_stream *stream, const char *buf, size_t count)
+static ssize_t php_bz2iop_write(php_stream *stream, const char *buf, size_t count)
 {
-	size_t wrote = 0;
+	ssize_t wrote = 0;
 	struct php_bz2_stream_data_t *self = (struct php_bz2_stream_data_t *)stream->abstract;
 
-
 	do {
 		int just_wrote;
 		size_t remain = count - wrote;
 		int to_write = (int)(remain <= INT_MAX ? remain : INT_MAX);
 
 		just_wrote = BZ2_bzwrite(self->bz_file, (char*)buf, to_write);
-
-		if (just_wrote < 1) {
+		if (just_wrote < 0) {
+			if (wrote == 0) {
+				return just_wrote;
+			}
+			return wrote;
+		}
+		if (just_wrote == 0) {
 			break;
 		}
 
@@ -381,11 +388,12 @@ static PHP_FUNCTION(bzread)
 		php_error_docref(NULL, E_WARNING, "length may not be negative");
 		RETURN_FALSE;
 	}
-	data = zend_string_alloc(len, 0);
-	ZSTR_LEN(data) = php_stream_read(stream, ZSTR_VAL(data), ZSTR_LEN(data));
-	ZSTR_VAL(data)[ZSTR_LEN(data)] = '\0';
 
-	RETURN_NEW_STR(data);
+	data = php_stream_read_to_str(stream, len);
+	if (!data) {
+		RETURN_FALSE;
+	}
+	RETURN_STR(data);
 }
 /* }}} */
 

ext/bz2/tests/004.phpt

@@ -60,7 +60,7 @@ array(2) {
 }
 string(2) "OK"
 int(0)
-string(0) ""
+bool(false)
 array(2) {
   ["errno"]=>
   int(-5)
@@ -69,7 +69,7 @@ array(2) {
 }
 string(16) "DATA_ERROR_MAGIC"
 int(-5)
-string(0) ""
+bool(false)
 array(2) {
   ["errno"]=>
   int(-4)
@@ -78,7 +78,7 @@ array(2) {
 }
 string(10) "DATA_ERROR"
 int(-4)
-string(0) ""
+bool(false)
 array(2) {
   ["errno"]=>
   int(-5)
@@ -87,7 +87,7 @@ array(2) {
 }
 string(16) "DATA_ERROR_MAGIC"
 int(-5)
-string(0) ""
+bool(false)
 array(2) {
   ["errno"]=>
   int(-4)

ext/bz2/tests/bug72613.phpt

@@ -20,4 +20,4 @@ bzclose($fp);
 ?>
 DONE
 --EXPECT--
-DONE
+ERROR: bzread()

ext/com_dotnet/com_persist.c

@@ -118,13 +118,13 @@ static HRESULT STDMETHODCALLTYPE stm_read(IStream *This, void *pv, ULONG cb, ULO
 
 static HRESULT STDMETHODCALLTYPE stm_write(IStream *This, void const *pv, ULONG cb, ULONG *pcbWritten)
 {
-	ULONG nwrote;
+	ssize_t nwrote;
 	FETCH_STM();
 
-	nwrote = (ULONG)php_stream_write(stm->stream, pv, cb);
+	nwrote = php_stream_write(stm->stream, pv, cb);
 
 	if (pcbWritten) {
-		*pcbWritten = nwrote > 0 ? nwrote : 0;
+		*pcbWritten = nwrote > 0 ? (ULONG)nwrote : 0;
 	}
 	if (nwrote > 0) {
 		return S_OK;

ext/curl/interface.c

@@ -2133,9 +2133,9 @@ PHP_FUNCTION(curl_copy_handle)
 static size_t read_cb(char *buffer, size_t size, size_t nitems, void *arg) /* {{{ */
 {
 	php_stream *stream = (php_stream *) arg;
-	size_t numread = php_stream_read(stream, buffer, nitems * size);
+	ssize_t numread = php_stream_read(stream, buffer, nitems * size);
 
-	if (numread == (size_t)-1) {
+	if (numread < 0) {
 		return CURL_READFUNC_ABORT;
 	}
 	return numread;

ext/exif/exif.c

@@ -3259,7 +3259,7 @@ static int exif_process_IFD_TAG(image_info_type *ImageInfo, char *dir_entry, cha
 			}
 			fgot = php_stream_read(ImageInfo->infile, value_ptr, byte_count);
 			php_stream_seek(ImageInfo->infile, fpos, SEEK_SET);
-			if (fgot<byte_count) {
+			if (fgot != byte_count) {
 				EFREE_IF(outside);
 				EXIF_ERRLOG_FILEEOF(ImageInfo)
 				return FALSE;
@@ -4088,7 +4088,7 @@ static int exif_process_IFD_in_TIFF(image_info_type *ImageInfo, size_t dir_offse
 									ImageInfo->Thumbnail.data = safe_emalloc(ImageInfo->Thumbnail.size, 1, 0);
 									php_stream_seek(ImageInfo->infile, ImageInfo->Thumbnail.offset, SEEK_SET);
 									fgot = php_stream_read(ImageInfo->infile, ImageInfo->Thumbnail.data, ImageInfo->Thumbnail.size);
-									if (fgot < ImageInfo->Thumbnail.size) {
+									if (fgot != ImageInfo->Thumbnail.size) {
 										EXIF_ERRLOG_THUMBEOF(ImageInfo)
 										efree(ImageInfo->Thumbnail.data);
 
@@ -4126,7 +4126,7 @@ static int exif_process_IFD_in_TIFF(image_info_type *ImageInfo, size_t dir_offse
 						ImageInfo->Thumbnail.data = safe_emalloc(ImageInfo->Thumbnail.size, 1, 0);
 						php_stream_seek(ImageInfo->infile, ImageInfo->Thumbnail.offset, SEEK_SET);
 						fgot = php_stream_read(ImageInfo->infile, ImageInfo->Thumbnail.data, ImageInfo->Thumbnail.size);
-						if (fgot < ImageInfo->Thumbnail.size) {
+						if (fgot != ImageInfo->Thumbnail.size) {
 							EXIF_ERRLOG_THUMBEOF(ImageInfo)
 							efree(ImageInfo->Thumbnail.data);
 							ImageInfo->Thumbnail.data = NULL;

ext/gd/gd.c

@@ -1441,11 +1441,11 @@ PHP_FUNCTION(imageloadfont)
 	 */
 	font = (gdFontPtr) emalloc(sizeof(gdFont));
 	b = 0;
-	while (b < hdr_size && (n = php_stream_read(stream, (char*)&font[b], hdr_size - b))) {
+	while (b < hdr_size && (n = php_stream_read(stream, (char*)&font[b], hdr_size - b)) > 0) {
 		b += n;
 	}
 
-	if (!n) {
+	if (n <= 0) {
 		efree(font);
 		if (php_stream_eof(stream)) {
 			php_error_docref(NULL, E_WARNING, "End of file while reading header");
@@ -1484,11 +1484,11 @@ PHP_FUNCTION(imageloadfont)
 
 	font->data = emalloc(body_size);
 	b = 0;
-	while (b < body_size && (n = php_stream_read(stream, &font->data[b], body_size - b))) {
+	while (b < body_size && (n = php_stream_read(stream, &font->data[b], body_size - b)) > 0) {
 		b += n;
 	}
 
-	if (!n) {
+	if (n <= 0) {
 		efree(font->data);
 		efree(font);
 		if (php_stream_eof(stream)) {

ext/hash/hash.c

@@ -149,12 +149,16 @@ static void php_hash_do_hash(INTERNAL_FUNCTION_PARAMETERS, int isfilename, zend_
 
 	if (isfilename) {
 		char buf[1024];
-		size_t n;
+		ssize_t n;
 
 		while ((n = php_stream_read(stream, buf, sizeof(buf))) > 0) {
 			ops->hash_update(context, (unsigned char *) buf, n);
 		}
 		php_stream_close(stream);
+		if (n < 0) {
+			efree(context);
+			RETURN_FALSE;
+		}
 	} else {
 		ops->hash_update(context, (unsigned char *) data, data_len);
 	}
@@ -277,13 +281,20 @@ static void php_hash_do_hash_hmac(INTERNAL_FUNCTION_PARAMETERS, int isfilename,
 
 	if (isfilename) {
 		char buf[1024];
-		size_t n;
+		ssize_t n;
 		ops->hash_init(context);
 		ops->hash_update(context, K, ops->block_size);
 		while ((n = php_stream_read(stream, buf, sizeof(buf))) > 0) {
 			ops->hash_update(context, (unsigned char *) buf, n);
 		}
 		php_stream_close(stream);
+		if (n < 0) {
+			efree(context);
+			efree(K);
+			zend_string_release(digest);
+			RETURN_FALSE;
+		}
+
 		ops->hash_final((unsigned char *) ZSTR_VAL(digest), context);
 	} else {
 		php_hash_hmac_round((unsigned char *) ZSTR_VAL(digest), ops, context, K, (unsigned char *) data, data_len);
@@ -447,14 +458,14 @@ PHP_FUNCTION(hash_update_stream)
 
 	while (length) {
 		char buf[1024];
-		zend_long n, toread = 1024;
+		zend_long toread = 1024;
+		ssize_t n;
 
 		if (length > 0 && toread > length) {
 			toread = length;
 		}
 
 		if ((n = php_stream_read(stream, buf, toread)) <= 0) {
-			/* Nada mas */
 			RETURN_LONG(didread);
 		}
 		hash->ops->hash_update(hash->context, (unsigned char *) buf, n);
@@ -476,7 +487,7 @@ PHP_FUNCTION(hash_update_file)
 	php_stream *stream;
 	zend_string *filename;
 	char buf[1024];
-	size_t n;
+	ssize_t n;
 
 	if (zend_parse_parameters(ZEND_NUM_ARGS(), "OP|r", &zhash, php_hashcontext_ce, &filename, &zcontext) == FAILURE) {
 		return;
@@ -497,7 +508,7 @@ PHP_FUNCTION(hash_update_file)
 	}
 	php_stream_close(stream);
 
-	RETURN_TRUE;
+	RETURN_BOOL(n >= 0);
 }
 /* }}} */
 

ext/iconv/tests/bug76249.phpt

@@ -10,11 +10,10 @@ rewind($fh);
 if (false === @stream_filter_append($fh, 'convert.iconv.ucs-2/utf8//IGNORE', STREAM_FILTER_READ, [])) {
 	stream_filter_append($fh, 'convert.iconv.ucs-2/utf-8//IGNORE', STREAM_FILTER_READ, []);
 }
-$a = stream_get_contents($fh);
-var_dump(strlen($a));
+var_dump(stream_get_contents($fh));
 ?>
 DONE
 --EXPECTF--
 Warning: stream_get_contents(): iconv stream filter ("ucs-2"=>"utf%A8//IGNORE"): invalid multibyte sequence in %sbug76249.php on line %d
-int(3)
+string(0) ""
 DONE