[PECL-CVS] [pecl-text-xdiff] main: Merge pull request #10 from php/fix-memory-issues

Author: Rasmus Lerdorf (rlerdorf)
Committer: GitHub (web-flow)
Pusher: rlerdorf
Date: 2026-04-06T10:39:29+01:00

Commit: https://github.com/php/pecl-text-xdiff/commit/db90b2d8751a9c66ee806d161dfdb3f3ccfab09e
Raw diff: https://github.com/php/pecl-text-xdiff/commit/db90b2d8751a9c66ee806d161dfdb3f3ccfab09e.diff

Merge pull request #10 from php/fix-memory-issues

memory safety cleanup

Changed paths:
  A  tests/file_bpatch_retval.phpt
  M  xdiff.c


Diff:

diff --git a/tests/file_bpatch_retval.phpt b/tests/file_bpatch_retval.phpt
new file mode 100644
index 0000000..e0f8eda
--- /dev/null
+++ b/tests/file_bpatch_retval.phpt
@@ -0,0 +1,21 @@
+--TEST--
+xdiff_file_bpatch() return value on success and failure
+--SKIPIF--
+<?php if (!extension_loaded("xdiff")) print "skip"; ?>
+--FILE--
+<?php
+$result = xdiff_file_bpatch(__DIR__ . '/file.1', __DIR__ . '/file.bdiff', __DIR__ . '/file.bpatch_out');
+var_dump($result);
+$a = file_get_contents(__DIR__ . '/file.2');
+$b = file_get_contents(__DIR__ . '/file.bpatch_out');
+echo "content match: " . (strcmp($a, $b) === 0 ? "yes" : "no") . "\n";
+unlink(__DIR__ . '/file.bpatch_out');
+
+$result = xdiff_file_bpatch(__DIR__ . '/file.1', __DIR__ . '/file.1', __DIR__ . '/file.bpatch_out2');
+var_dump($result);
+@unlink(__DIR__ . '/file.bpatch_out2');
+?>
+--EXPECT--
+bool(true)
+content match: yes
+bool(false)
diff --git a/xdiff.c b/xdiff.c
index 9b0f8c3..72423fe 100644
--- a/xdiff.c
+++ b/xdiff.c
@@ -71,28 +71,28 @@ extern char libxdiff_version[];
 
 struct string_buffer {
 	char *ptr;
-	unsigned long size;
+	size_t size;
 };
 
 static int load_mm_file(const char *filepath, mmfile_t *dest);
-static int load_into_mm_file(const char *buffer, unsigned long size, mmfile_t *dest);
+static int load_into_mm_file(const char *buffer, size_t size, mmfile_t *dest);
 static int append_string(void *ptr, mmbuffer_t *buffer, int array_size);
 static int append_stream(void *ptr, mmbuffer_t *buffer, int array_size);
 static int init_string(struct string_buffer *string);
 static void free_string(struct string_buffer *string);
 
 static int make_diff(char *filepath1, char *filepath2, xdemitcb_t *output, int context, int minimal);
-static int make_diff_str(char *str1, int size1, char *str2, int size2,  xdemitcb_t *output, int context, int minimal);
+static int make_diff_str(char *str1, size_t size1, char *str2, size_t size2,  xdemitcb_t *output, int context, int minimal);
 static int make_bdiff(char *filepath1, char *filepath2, xdemitcb_t *output);
-static int make_bdiff_str(char *str1, int size1, char *str2, int size2, xdemitcb_t *output);
+static int make_bdiff_str(char *str1, size_t size1, char *str2, size_t size2, xdemitcb_t *output);
 static int make_patch(char *file_path, char *patch_path, xdemitcb_t *output, xdemitcb_t *error, int flags);
-static int make_patch_str(char *file, int size1, char *patch, int size2, xdemitcb_t *output, xdemitcb_t *error, int flags);
+static int make_patch_str(char *file, size_t size1, char *patch, size_t size2, xdemitcb_t *output, xdemitcb_t *error, int flags);
 static int make_bpatch(char *file_path, char *patch_path, xdemitcb_t *output);
-static int make_bpatch_str(char *file, int size1, char *patch, int size2, xdemitcb_t *output);
+static int make_bpatch_str(char *file, size_t size1, char *patch, size_t size2, xdemitcb_t *output);
 static int make_merge3(char *filepath1, char *filepath2, char *filepath3, xdemitcb_t *output, xdemitcb_t *error);
-static int make_merge3_str(char *content1, int size1, char *content2, int size2, char *content3, int size3, xdemitcb_t *output, xdemitcb_t *error);
+static int make_merge3_str(char *content1, size_t size1, char *content2, size_t size2, char *content3, size_t size3, xdemitcb_t *output, xdemitcb_t *error);
 static int make_rabdiff(char *filepath1, char *filepath2, xdemitcb_t *output);
-static int make_rabdiff_str(char *str1, int size1, char *str2, int size2, xdemitcb_t *output);
+static int make_rabdiff_str(char *str1, size_t size1, char *str2, size_t size2, xdemitcb_t *output);
 
 static void *xdiff_malloc(void *foo, unsigned int size)
 {
@@ -187,8 +187,13 @@ PHP_FUNCTION(xdiff_string_diff)
 	output.priv= &string;
 	output.outf = append_string;
 
-	make_diff_str(str1->val, str1->len, str2->val, str2->len, &output, context, minimal);
+	retval = make_diff_str(str1->val, str1->len, str2->val, str2->len, &output, context, minimal);
+	if (!retval)
+		goto out_free_string;
+
 	RETVAL_STRINGL(string.ptr, string.size);
+
+out_free_string:
 	free_string(&string);
 out:
 	return;
@@ -254,10 +259,14 @@ PHP_FUNCTION(xdiff_string_bdiff)
 	output.priv= &string;
 	output.outf = append_string;
 
-	make_bdiff_str(str1->val, str1->len, str2->val, str2->len, &output);
+	retval = make_bdiff_str(str1->val, str1->len, str2->val, str2->len, &output);
+	if (!retval)
+		goto out_free_string;
+
 	RETVAL_STRINGL(string.ptr, string.size);
-	free_string(&string);
 
+out_free_string:
+	free_string(&string);
 out:
 	return;
 }
@@ -320,10 +329,14 @@ PHP_FUNCTION(xdiff_string_rabdiff)
 	output.priv= &string;
 	output.outf = append_string;
 
-	make_rabdiff_str(str1->val, str1->len, str2->val, str2->len, &output);
+	retval = make_rabdiff_str(str1->val, str1->len, str2->val, str2->len, &output);
+	if (!retval)
+		goto out_free_string;
+
 	RETVAL_STRINGL(string.ptr, string.size);
-	free_string(&string);
 
+out_free_string:
+	free_string(&string);
 out:
 	return;
 }
@@ -557,7 +570,7 @@ PHP_FUNCTION(xdiff_file_bpatch)
 	retval = make_bpatch(src_path->val, patch_path->val, &output);
 	php_stream_close(output_stream);
 
-	if (retval == 0)
+	if (retval)
 		RETVAL_TRUE;
 
 out:
@@ -719,8 +732,10 @@ static int load_mm_file(const char *filepath, mmfile_t *dest)
 		goto out_stream_close;
 
 	filesize = stat.sb.st_size;
+	if (filesize < 0 || filesize > LONG_MAX)
+		goto out_stream_close;
 
-	retval = xdl_init_mmfile(dest, filesize, XDL_MMF_ATOMIC);
+	retval = xdl_init_mmfile(dest, (long) filesize, XDL_MMF_ATOMIC);
 	if (retval < 0)
 		goto out_stream_close;
 
@@ -728,7 +743,9 @@ static int load_mm_file(const char *filepath, mmfile_t *dest)
 	if (!ptr)
 		goto out_free_mmfile;
 
-	php_stream_read(src, ptr, filesize);
+	if (php_stream_read(src, ptr, filesize) != filesize)
+		goto out_free_mmfile;
+
 	php_stream_close(src);
 
 	return 1;
@@ -741,12 +758,15 @@ static int load_mm_file(const char *filepath, mmfile_t *dest)
 	return 0;
 }
 
-static int load_into_mm_file(const char *buffer, unsigned long size, mmfile_t *dest)
+static int load_into_mm_file(const char *buffer, size_t size, mmfile_t *dest)
 {
 	int retval;
 	void *ptr;
 
-	retval = xdl_init_mmfile(dest, size, XDL_MMF_ATOMIC);
+	if (size > LONG_MAX)
+		goto out;
+
+	retval = xdl_init_mmfile(dest, (long) size, XDL_MMF_ATOMIC);
 	if (retval < 0)
 		goto out;
 
@@ -767,12 +787,21 @@ static int append_string(void *ptr, mmbuffer_t *buffer, int array_size)
 {
 	struct string_buffer *string = ptr;
 	void *new_ptr;
-	unsigned int i;
+	int i;
+
+	if (array_size <= 0)
+		return 0;
 
 	for (i = 0; i < array_size; i++) {
+		if (buffer[i].size < 0 || (size_t) buffer[i].size > SIZE_MAX - string->size - 1) {
+			efree(string->ptr);
+			string->ptr = NULL;
+			return -1;
+		}
 		new_ptr = erealloc(string->ptr, string->size + buffer[i].size + 1);
 		if (!new_ptr) {
 			efree(string->ptr);
+			string->ptr = NULL;
 			return -1;
 		}
 
@@ -780,9 +809,7 @@ static int append_string(void *ptr, mmbuffer_t *buffer, int array_size)
 		memcpy(string->ptr + string->size, buffer[i].ptr, buffer[i].size);
 		string->size += buffer[i].size;
 	}
-	if (array_size) {
-		string->ptr[string->size] = '\0';
-	}
+	string->ptr[string->size] = '\0';
 
 	return 0;
 }
@@ -790,7 +817,10 @@ static int append_string(void *ptr, mmbuffer_t *buffer, int array_size)
 static int append_stream(void *ptr, mmbuffer_t *buffer, int array_size)
 {
 	php_stream *stream = ptr;
-	unsigned int i;
+	int i;
+
+	if (array_size <= 0)
+		return 1;
 
 	for (i = 0; i < array_size; i++) {
 		php_stream_write(stream, buffer[i].ptr, buffer[i].size);
@@ -849,7 +879,7 @@ static int make_diff(char *filepath1, char *filepath2, xdemitcb_t *output, int c
 	return result;
 }
 
-static int make_diff_str(char *str1, int size1, char *str2, int size2, xdemitcb_t *output, int context, int minimal)
+static int make_diff_str(char *str1, size_t size1, char *str2, size_t size2, xdemitcb_t *output, int context, int minimal)
 {
 	mmfile_t file1, file2;
 	xpparam_t params;
@@ -911,7 +941,7 @@ static int make_bdiff(char *filepath1, char *filepath2, xdemitcb_t *output)
 	return result;
 }
 
-static int make_bdiff_str(char *str1, int size1, char *str2, int size2, xdemitcb_t *output)
+static int make_bdiff_str(char *str1, size_t size1, char *str2, size_t size2, xdemitcb_t *output)
 {
 	mmfile_t file1, file2;
 	bdiffparam_t params;
@@ -968,7 +998,7 @@ static int make_rabdiff(char *filepath1, char *filepath2, xdemitcb_t *output)
 	return result;
 }
 
-static int make_rabdiff_str(char *str1, int size1, char *str2, int size2, xdemitcb_t *output)
+static int make_rabdiff_str(char *str1, size_t size1, char *str2, size_t size2, xdemitcb_t *output)
 {
 	mmfile_t file1, file2;
 	int retval, result = 0;
@@ -1022,7 +1052,7 @@ static int make_patch(char *file_path, char *patch_path, xdemitcb_t *output, xde
 	return result;
 }
 
-static int make_patch_str(char *file, int size1, char *patch, int size2, xdemitcb_t *output, xdemitcb_t *error, int flags)
+static int make_patch_str(char *file, size_t size1, char *patch, size_t size2, xdemitcb_t *output, xdemitcb_t *error, int flags)
 {
 	mmfile_t file_mm, patch_mm;
 	int retval, result = 0;
@@ -1076,7 +1106,7 @@ static int make_bpatch(char *file_path, char *patch_path, xdemitcb_t *output)
 	return result;
 }
 
-static int make_bpatch_str(char *file, int size1, char *patch, int size2, xdemitcb_t *output)
+static int make_bpatch_str(char *file, size_t size1, char *patch, size_t size2, xdemitcb_t *output)
 {
 	mmfile_t file_mm, patch_mm;
 	int retval, result = 0;
@@ -1136,7 +1166,7 @@ static int make_merge3(char *filepath1, char *filepath2, char *filepath3, xdemit
 	return result;
 }
 
-static int make_merge3_str(char *content1, int size1, char *content2, int size2, char *content3, int size3, xdemitcb_t *output, xdemitcb_t *error)
+static int make_merge3_str(char *content1, size_t size1, char *content2, size_t size2, char *content3, size_t size3, xdemitcb_t *output, xdemitcb_t *error)
 {
 	mmfile_t file1, file2, file3;
 	int retval, result = 0;