Skip to content

Commit a7c8789

Browse files
committed
fix possible file descriptor leaks
1 parent a9fe4e6 commit a7c8789

File tree

1 file changed

+43
-10
lines changed

1 file changed

+43
-10
lines changed

php_bsdiff.c

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -102,18 +102,22 @@ PHP_FUNCTION(bsdiff_diff)
102102
RETURN_THROWS();
103103
}
104104
if ((oldsize = lseek(fd, 0, SEEK_END)) == -1) {
105+
close(fd);
105106
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to determine size of the old file \"%s\"", old_file);
106107
RETURN_THROWS();
107108
}
108109
if ((old = malloc(oldsize + 1)) == NULL) {
110+
close(fd);
109111
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to allocate memory to store old data");
110112
RETURN_THROWS();
111113
}
112-
if ((lseek(fd, 0, SEEK_SET) != 0) || (read(fd, old, oldsize) != oldsize) || (close(fd) == -1)) {
114+
if ((lseek(fd, 0, SEEK_SET) != 0) || (read(fd, old, oldsize) != oldsize)) {
113115
free(old);
116+
close(fd);
114117
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to read data from the old file \"%s\"", old_file);
115118
RETURN_THROWS();
116119
}
120+
close(fd);
117121

118122
/* Allocate newsize+1 bytes instead of newsize bytes to ensure that we never try to malloc(0) and get a NULL pointer */
119123
if ((fd = open(new_file, O_RDONLY, 0)) < 0) {
@@ -123,20 +127,24 @@ PHP_FUNCTION(bsdiff_diff)
123127
}
124128
if ((newsize = lseek(fd, 0, SEEK_END)) == -1) {
125129
free(old);
130+
close(fd);
126131
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to determine size of the new file \"%s\"", new_file);
127132
RETURN_THROWS();
128133
}
129134
if ((new = malloc(newsize + 1)) == NULL) {
130135
free(old);
136+
close(fd);
131137
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to allocate memory to store new data");
132138
RETURN_THROWS();
133139
}
134-
if ((lseek(fd, 0, SEEK_SET) != 0) || (read(fd, new, newsize) != newsize) || (close(fd) == -1)) {
140+
if ((lseek(fd, 0, SEEK_SET) != 0) || (read(fd, new, newsize) != newsize)) {
135141
free(old);
136142
free(new);
143+
close(fd);
137144
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to read data from the new file \"%s\"", new_file);
138145
RETURN_THROWS();
139146
}
147+
close(fd);
140148

141149
/* Create the patch file */
142150
if ((pf = fopen(diff_file, "w")) == NULL) {
@@ -152,6 +160,7 @@ PHP_FUNCTION(bsdiff_diff)
152160
fwrite(buf, sizeof(buf), 1, pf) != 1) {
153161
free(old);
154162
free(new);
163+
fclose(pf);
155164
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to write header to the diff file");
156165
RETURN_THROWS();
157166
}
@@ -160,6 +169,7 @@ PHP_FUNCTION(bsdiff_diff)
160169
if (NULL == (bz2 = BZ2_bzWriteOpen(&bz2err, pf, 9, 0, 0))) {
161170
free(old);
162171
free(new);
172+
fclose(pf);
163173
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to prepare to write data to the diff file (bz2err=%d)", bz2err);
164174
RETURN_THROWS();
165175
}
@@ -168,6 +178,8 @@ PHP_FUNCTION(bsdiff_diff)
168178
if (bsdiff(old, oldsize, new, newsize, &stream)) {
169179
free(old);
170180
free(new);
181+
BZ2_bzWriteClose(&bz2err, bz2, 0, NULL, NULL);
182+
fclose(pf);
171183
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to create diff data");
172184
RETURN_THROWS();
173185
}
@@ -178,14 +190,12 @@ PHP_FUNCTION(bsdiff_diff)
178190

179191
BZ2_bzWriteClose(&bz2err, bz2, 0, NULL, NULL);
180192
if (bz2err != BZ_OK) {
193+
fclose(pf);
181194
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to complete writing data to the diff file (bz2err=%d)", bz2err);
182195
RETURN_THROWS();
183196
}
184197

185-
if (fclose(pf)) {
186-
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to close the diff file");
187-
RETURN_THROWS();
188-
}
198+
fclose(pf);
189199
}
190200
/* }}} */
191201

@@ -220,55 +230,69 @@ PHP_FUNCTION(bsdiff_patch)
220230
/* Read header */
221231
if (fread(header, 1, 24, f) != 24) {
222232
if (feof(f)) {
233+
fclose(f);
223234
zend_throw_exception_ex(ce_bsdiff_exception, 0, "The diff file is corrupted (missing header information)");
224235
RETURN_THROWS();
225236
}
237+
fclose(f);
226238
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to read data from the diff file");
227239
RETURN_THROWS();
228240
}
229241

230242
/* Check for appropriate magic */
231243
if (memcmp(header, "ENDSLEY/BSDIFF43", 16) != 0) {
244+
fclose(f);
232245
zend_throw_exception_ex(ce_bsdiff_exception, 0, "The diff file is corrupted (invalid header information)");
233246
RETURN_THROWS();
234247
}
235248

236249
/* Read lengths from header */
237250
newsize=offtin(header+16);
238251
if(newsize<0) {
252+
fclose(f);
239253
zend_throw_exception_ex(ce_bsdiff_exception, 0, "The diff file is corrupted (invalid length information)");
240254
RETURN_THROWS();
241255
}
242256

243257
/* Close patch file and re-open it via libbzip2 at the right places */
244258
if ((fd = open(old_file, O_RDONLY, 0)) < 0) {
259+
fclose(f);
245260
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to open the old file \"%s\"", old_file);
246261
RETURN_THROWS();
247262
}
248263
if ((oldsize = lseek(fd, 0, SEEK_END)) == -1) {
264+
fclose(f);
265+
close(fd);
249266
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to determine size of the old file \"%s\"", old_file);
250267
RETURN_THROWS();
251268
}
252269
if ((old = malloc(oldsize + 1)) == NULL) {
270+
fclose(f);
271+
close(fd);
253272
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to allocate memory to store old data");
254273
RETURN_THROWS();
255274
}
256-
if ((lseek(fd, 0, SEEK_SET) != 0) || (read(fd, old, oldsize) != oldsize) || (fstat(fd, &sb)) || (close(fd) == -1)) {
275+
if ((lseek(fd, 0, SEEK_SET) != 0) || (read(fd, old, oldsize) != oldsize) || (fstat(fd, &sb))) {
257276
free(old);
277+
fclose(f);
278+
close(fd);
258279
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to read data from the old file \"%s\"", old_file);
259280
RETURN_THROWS();
260281
}
282+
close(fd);
261283

262284
if((new=malloc(newsize+1))==NULL) {
263285
free(old);
286+
fclose(f);
264287
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to allocate memory to store patched data");
265288
RETURN_THROWS();
266289
}
267290

268291
if (NULL == (bz2 = BZ2_bzReadOpen(&bz2err, f, 0, 0, NULL, 0))) {
269-
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to read data from the diff file (bz2err=%d)", bz2err);
270292
free(new);
271293
free(old);
294+
fclose(f);
295+
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to read data from the diff file (bz2err=%d)", bz2err);
272296
RETURN_THROWS();
273297
}
274298

@@ -277,6 +301,8 @@ PHP_FUNCTION(bsdiff_patch)
277301
if (bspatch(old, oldsize, new, newsize, &stream)) {
278302
free(new);
279303
free(old);
304+
BZ2_bzReadClose(&bz2err, bz2);
305+
fclose(f);
280306
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to apply diff data");
281307
RETURN_THROWS();
282308
}
@@ -286,16 +312,23 @@ PHP_FUNCTION(bsdiff_patch)
286312
fclose(f);
287313

288314
/* Write the new file */
289-
if(((fd=open(new_file,O_CREAT|O_TRUNC|O_WRONLY,sb.st_mode))<0) ||
290-
(write(fd,new,newsize)!=newsize) || (close(fd)==-1)) {
315+
if ((fd = open(new_file, O_CREAT | O_TRUNC | O_WRONLY, sb.st_mode)) < 0) {
316+
free(new);
317+
free(old);
318+
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to create the new file \"%s\"", new_file);
319+
RETURN_THROWS();
320+
}
321+
if (write(fd,new,newsize) != newsize) {
291322
free(new);
292323
free(old);
324+
close(fd);
293325
zend_throw_exception_ex(ce_bsdiff_exception, 0, "Failed to create the new file \"%s\"", new_file);
294326
RETURN_THROWS();
295327
}
296328

297329
free(new);
298330
free(old);
331+
close(fd);
299332
}
300333
/* }}}*/
301334

0 commit comments

Comments
 (0)