Skip to content

Commit f911877

Browse files
deminydeminy
committed
fix possible memory leaks
1 parent 24144eb commit f911877

File tree

3 files changed

+71
-30
lines changed

3 files changed

+71
-30
lines changed

‎php_bsdiff.c‎

Lines changed: 68 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -96,32 +96,52 @@ PHP_FUNCTION(bsdiff_diff)
9696
stream.free = free;
9797
stream.write = bz2_write;
9898

99-
/* Allocate oldsize+1 bytes instead of oldsize bytes to ensure
100-
that we never try to malloc(0) and get a NULL pointer */
101-
if(((fd=open(old_file,O_RDONLY,0))<0) ||
102-
((oldsize=lseek(fd,0,SEEK_END))==-1) ||
103-
((old=malloc(oldsize+1))==NULL) ||
104-
(lseek(fd,0,SEEK_SET)!=0) ||
105-
(read(fd,old,oldsize)!=oldsize) ||
106-
(close(fd)==-1)) {
99+
/* Allocate oldsize+1 bytes instead of oldsize bytes to ensure that we never try to malloc(0) and get a NULL pointer */
100+
if ((fd = open(old_file, O_RDONLY, 0)) < 0) {
101+
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to open the old file \"%s\"", old_file);
102+
RETURN_THROWS();
103+
}
104+
if ((oldsize = lseek(fd, 0, SEEK_END)) == -1) {
105+
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to determine size of the old file \"%s\"", old_file);
106+
RETURN_THROWS();
107+
}
108+
if ((old = malloc(oldsize + 1)) == NULL) {
109+
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to allocate memory to store old data");
110+
RETURN_THROWS();
111+
}
112+
if ((lseek(fd, 0, SEEK_SET) != 0) || (read(fd, old, oldsize) != oldsize) || (close(fd) == -1)) {
113+
free(old);
107114
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to read data from the old file \"%s\"", old_file);
108115
RETURN_THROWS();
109116
}
110117

111-
/* Allocate newsize+1 bytes instead of newsize bytes to ensure
112-
that we never try to malloc(0) and get a NULL pointer */
113-
if(((fd=open(new_file,O_RDONLY,0))<0) ||
114-
((newsize=lseek(fd,0,SEEK_END))==-1) ||
115-
((new=malloc(newsize+1))==NULL) ||
116-
(lseek(fd,0,SEEK_SET)!=0) ||
117-
(read(fd,new,newsize)!=newsize) ||
118-
(close(fd)==-1)) {
118+
/* Allocate newsize+1 bytes instead of newsize bytes to ensure that we never try to malloc(0) and get a NULL pointer */
119+
if ((fd = open(new_file, O_RDONLY, 0)) < 0) {
120+
free(old);
121+
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to open the new file \"%s\"", new_file);
122+
RETURN_THROWS();
123+
}
124+
if ((newsize = lseek(fd, 0, SEEK_END)) == -1) {
125+
free(old);
126+
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to determine size of the new file \"%s\"", new_file);
127+
RETURN_THROWS();
128+
}
129+
if ((new = malloc(newsize + 1)) == NULL) {
130+
free(old);
131+
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to allocate memory to store new data");
132+
RETURN_THROWS();
133+
}
134+
if ((lseek(fd, 0, SEEK_SET) != 0) || (read(fd, new, newsize) != newsize) || (close(fd) == -1)) {
135+
free(old);
136+
free(new);
119137
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to read data from the new file \"%s\"", new_file);
120138
RETURN_THROWS();
121139
}
122140

123141
/* Create the patch file */
124142
if ((pf = fopen(diff_file, "w")) == NULL) {
143+
free(old);
144+
free(new);
125145
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Cannot open the diff file \"%s\" in write mode", diff_file);
126146
RETURN_THROWS();
127147
}
@@ -130,22 +150,32 @@ PHP_FUNCTION(bsdiff_diff)
130150
offtout(newsize, buf);
131151
if (fwrite("ENDSLEY/BSDIFF43", 16, 1, pf) != 1 ||
132152
fwrite(buf, sizeof(buf), 1, pf) != 1) {
153+
free(old);
154+
free(new);
133155
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to write header to the diff file");
134156
RETURN_THROWS();
135157
}
136158

137159

138160
if (NULL == (bz2 = BZ2_bzWriteOpen(&bz2err, pf, 9, 0, 0))) {
161+
free(old);
162+
free(new);
139163
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to prepare to write data to the diff file (bz2err=%d)", bz2err);
140164
RETURN_THROWS();
141165
}
142166

143167
stream.opaque = bz2;
144168
if (bsdiff(old, oldsize, new, newsize, &stream)) {
169+
free(old);
170+
free(new);
145171
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to create diff data");
146172
RETURN_THROWS();
147173
}
148174

175+
/* Free the memory we used */
176+
free(old);
177+
free(new);
178+
149179
BZ2_bzWriteClose(&bz2err, bz2, 0, NULL, NULL);
150180
if (bz2err != BZ_OK) {
151181
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to complete writing data to the diff file (bz2err=%d)", bz2err);
@@ -156,10 +186,6 @@ PHP_FUNCTION(bsdiff_diff)
156186
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to close the diff file");
157187
RETURN_THROWS();
158188
}
159-
160-
/* Free the memory we used */
161-
free(old);
162-
free(new);
163189
}
164190
/* }}} */
165191

@@ -215,29 +241,42 @@ PHP_FUNCTION(bsdiff_patch)
215241
}
216242

217243
/* Close patch file and re-open it via libbzip2 at the right places */
218-
if(((fd=open(old_file,O_RDONLY,0))<0) ||
219-
((oldsize=lseek(fd,0,SEEK_END))==-1) ||
220-
((old=malloc(oldsize+1))==NULL) ||
221-
(lseek(fd,0,SEEK_SET)!=0) ||
222-
(read(fd,old,oldsize)!=oldsize) ||
223-
(fstat(fd, &sb)) ||
224-
(close(fd)==-1)) {
244+
if ((fd = open(old_file, O_RDONLY, 0)) < 0) {
245+
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to open the old file \"%s\"", old_file);
246+
RETURN_THROWS();
247+
}
248+
if ((oldsize = lseek(fd, 0, SEEK_END)) == -1) {
249+
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to determine size of the old file \"%s\"", old_file);
250+
RETURN_THROWS();
251+
}
252+
if ((old = malloc(oldsize + 1)) == NULL) {
253+
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to allocate memory to store old data");
254+
RETURN_THROWS();
255+
}
256+
if ((lseek(fd, 0, SEEK_SET) != 0) || (read(fd, old, oldsize) != oldsize) || (fstat(fd, &sb)) || (close(fd) == -1)) {
257+
free(old);
225258
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to read data from the old file \"%s\"", old_file);
226259
RETURN_THROWS();
227260
}
261+
228262
if((new=malloc(newsize+1))==NULL) {
263+
free(old);
229264
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to allocate memory to store patched data");
230265
RETURN_THROWS();
231266
}
232267

233268
if (NULL == (bz2 = BZ2_bzReadOpen(&bz2err, f, 0, 0, NULL, 0))) {
234269
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to read data from the diff file (bz2err=%d)", bz2err);
270+
free(new);
271+
free(old);
235272
RETURN_THROWS();
236273
}
237274

238275
stream.read = bz2_read;
239276
stream.opaque = bz2;
240277
if (bspatch(old, oldsize, new, newsize, &stream)) {
278+
free(new);
279+
free(old);
241280
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to apply diff data");
242281
RETURN_THROWS();
243282
}
@@ -249,6 +288,8 @@ PHP_FUNCTION(bsdiff_patch)
249288
/* Write the new file */
250289
if(((fd=open(new_file,O_CREAT|O_TRUNC|O_WRONLY,sb.st_mode))<0) ||
251290
(write(fd,new,newsize)!=newsize) || (close(fd)==-1)) {
291+
free(new);
292+
free(old);
252293
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to create the new file \"%s\"", new_file);
253294
RETURN_THROWS();
254295
}

‎tests/003_bsdiff_exceptions.phpt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,6 @@ try {
3535
}
3636
?>
3737
--EXPECTF--
38-
string(%d) "Failed to read data from the old file "%s/tests/003_old.out""
39-
string(%d) "Failed to read data from the new file "%s/tests/003_new.out""
38+
string(%d) "Failed to open the old file "%s/tests/003_old.out""
39+
string(%d) "Failed to open the new file "%s/tests/003_new.out""
4040
string(%d) "Cannot open the diff file "%s/tests/003_diff.out" in write mode"

‎tests/004_bspatch_exceptions.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,5 +57,5 @@ try {
5757
string(%d) "Cannot open diff file "%s/tests/004_diff.out" in read mode"
5858
string(%d) "The diff file is corrupted (missing header information)"
5959
string(%d) "The diff file is corrupted (invalid header information)"
60-
string(%d) "Failed to read data from the old file "%s/tests/004_old.out""
60+
string(%d) "Failed to open the old file "%s/tests/004_old.out""
6161
string(%d) "Failed to create the new file "%s/tests/004_patched.out""

0 commit comments

Comments
 (0)