Skip to content

Commit 8f71d79

Browse files
iliaaliliaal
committed
Fix GH-17399: iconv memory leak on bailout
Move the buf allocation in _php_iconv_mime_encode() before the iconv_open() calls so an OOM bailout from safe_emalloc does not leak iconv handles. Additionally, wrap bailable sections in php_iconv_string(), _php_iconv_substr(), _php_iconv_mime_encode(), and _php_iconv_mime_decode() with zend_try/zend_catch to ensure iconv handles allocated via system malloc are closed if a Zend OOM bailout fires during smart_str or zend_string operations. Closes GH-17399
1 parent b97dd33 commit 8f71d79

File tree

2 files changed

+141
-82
lines changed

2 files changed

+141
-82
lines changed

‎ext/iconv/iconv.c‎

Lines changed: 128 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -459,59 +459,65 @@ PHP_ICONV_API php_iconv_err_t php_iconv_string(const char *in_p, size_t in_len,
459459
out_left = in_len + 32; /* Avoid realloc() most cases */
460460
out_size = 0;
461461
bsz = out_left;
462-
out_buf = zend_string_alloc(bsz, 0);
463-
out_p = ZSTR_VAL(out_buf);
464-
465-
while (in_left > 0) {
466-
result = iconv(cd, (ICONV_CONST char **) &in_p, &in_left, (char **) &out_p, &out_left);
467-
out_size = bsz - out_left;
468-
if (result == (size_t)(-1)) {
469-
if (ignore_ilseq && errno == EILSEQ) {
470-
if (in_left <= 1) {
471-
result = 0;
472-
} else {
473-
errno = 0;
474-
in_p++;
475-
in_left--;
476-
continue;
462+
463+
zend_try {
464+
out_buf = zend_string_alloc(bsz, 0);
465+
out_p = ZSTR_VAL(out_buf);
466+
467+
while (in_left > 0) {
468+
result = iconv(cd, (ICONV_CONST char **) &in_p, &in_left, (char **) &out_p, &out_left);
469+
out_size = bsz - out_left;
470+
if (result == (size_t)(-1)) {
471+
if (ignore_ilseq && errno == EILSEQ) {
472+
if (in_left <= 1) {
473+
result = 0;
474+
} else {
475+
errno = 0;
476+
in_p++;
477+
in_left--;
478+
continue;
479+
}
477480
}
478-
}
479481

480-
if (errno == E2BIG && in_left > 0) {
481-
/* converted string is longer than out buffer */
482-
bsz += in_len;
482+
if (errno == E2BIG && in_left > 0) {
483+
/* converted string is longer than out buffer */
484+
bsz += in_len;
483485

484-
out_buf = zend_string_extend(out_buf, bsz, 0);
485-
out_p = ZSTR_VAL(out_buf);
486-
out_p += out_size;
487-
out_left = bsz - out_size;
488-
continue;
486+
out_buf = zend_string_extend(out_buf, bsz, 0);
487+
out_p = ZSTR_VAL(out_buf);
488+
out_p += out_size;
489+
out_left = bsz - out_size;
490+
continue;
491+
}
489492
}
493+
break;
490494
}
491-
break;
492-
}
493495

494-
if (result != (size_t)(-1)) {
495-
/* flush the shift-out sequences */
496-
for (;;) {
497-
result = iconv(cd, NULL, NULL, (char **) &out_p, &out_left);
498-
out_size = bsz - out_left;
496+
if (result != (size_t)(-1)) {
497+
/* flush the shift-out sequences */
498+
for (;;) {
499+
result = iconv(cd, NULL, NULL, (char **) &out_p, &out_left);
500+
out_size = bsz - out_left;
499501

500-
if (result != (size_t)(-1)) {
501-
break;
502-
}
502+
if (result != (size_t)(-1)) {
503+
break;
504+
}
503505

504-
if (errno == E2BIG) {
505-
bsz += 16;
506-
out_buf = zend_string_extend(out_buf, bsz, 0);
507-
out_p = ZSTR_VAL(out_buf);
508-
out_p += out_size;
509-
out_left = bsz - out_size;
510-
} else {
511-
break;
506+
if (errno == E2BIG) {
507+
bsz += 16;
508+
out_buf = zend_string_extend(out_buf, bsz, 0);
509+
out_p = ZSTR_VAL(out_buf);
510+
out_p += out_size;
511+
out_left = bsz - out_size;
512+
} else {
513+
break;
514+
}
512515
}
513516
}
514-
}
517+
} zend_catch {
518+
iconv_close(cd);
519+
zend_bailout();
520+
} zend_end_try();
515521

516522
iconv_close(cd);
517523

@@ -684,58 +690,68 @@ static php_iconv_err_t _php_iconv_substr(smart_str *pretval,
684690
errno = 0;
685691
more = nbytes > 0 && len > 0;
686692

687-
for (in_p = str, in_left = nbytes, cnt = 0; more; ++cnt) {
688-
out_p = buf;
689-
out_left = sizeof(buf);
693+
zend_try {
694+
for (in_p = str, in_left = nbytes, cnt = 0; more; ++cnt) {
695+
out_p = buf;
696+
out_left = sizeof(buf);
690697

691-
more = in_left > 0 && len > 0;
698+
more = in_left > 0 && len > 0;
692699

693-
iconv(cd1, more ? (ICONV_CONST char **)&in_p : NULL, more ? &in_left : NULL, (char **) &out_p, &out_left);
694-
if (out_left == sizeof(buf)) {
695-
break;
696-
}
700+
iconv(cd1, more ? (ICONV_CONST char **)&in_p : NULL, more ? &in_left : NULL, (char **) &out_p, &out_left);
701+
if (out_left == sizeof(buf)) {
702+
break;
703+
}
697704

698-
if ((zend_long)cnt >= offset) {
699-
if (cd2 == (iconv_t)NULL) {
700-
cd2 = iconv_open(enc, GENERIC_SUPERSET_NAME);
705+
if ((zend_long)cnt >= offset) {
706+
if (cd2 == (iconv_t)NULL) {
707+
cd2 = iconv_open(enc, GENERIC_SUPERSET_NAME);
701708

702-
if (cd2 == (iconv_t)(-1)) {
703-
cd2 = (iconv_t)NULL;
704-
if (errno == EINVAL) {
705-
err = PHP_ICONV_ERR_WRONG_CHARSET;
706-
} else {
707-
err = PHP_ICONV_ERR_CONVERTER;
709+
if (cd2 == (iconv_t)(-1)) {
710+
cd2 = (iconv_t)NULL;
711+
if (errno == EINVAL) {
712+
err = PHP_ICONV_ERR_WRONG_CHARSET;
713+
} else {
714+
err = PHP_ICONV_ERR_CONVERTER;
715+
}
716+
break;
708717
}
718+
}
719+
720+
if (_php_iconv_appendl(pretval, buf, sizeof(buf), cd2) != PHP_ICONV_ERR_SUCCESS) {
709721
break;
710722
}
723+
--len;
711724
}
712725

713-
if (_php_iconv_appendl(pretval, buf, sizeof(buf), cd2) != PHP_ICONV_ERR_SUCCESS) {
714-
break;
715-
}
716-
--len;
717726
}
718727

719-
}
720-
721-
switch (errno) {
722-
case EINVAL:
723-
err = PHP_ICONV_ERR_ILLEGAL_CHAR;
724-
break;
728+
switch (errno) {
729+
case EINVAL:
730+
err = PHP_ICONV_ERR_ILLEGAL_CHAR;
731+
break;
725732

726-
case EILSEQ:
727-
err = PHP_ICONV_ERR_ILLEGAL_SEQ;
728-
break;
733+
case EILSEQ:
734+
err = PHP_ICONV_ERR_ILLEGAL_SEQ;
735+
break;
729736

730-
case E2BIG:
731-
break;
732-
}
733-
if (err == PHP_ICONV_ERR_SUCCESS) {
737+
case E2BIG:
738+
break;
739+
}
740+
if (err == PHP_ICONV_ERR_SUCCESS) {
741+
if (cd2 != (iconv_t)NULL) {
742+
_php_iconv_appendl(pretval, NULL, 0, cd2);
743+
}
744+
smart_str_0(pretval);
745+
}
746+
} zend_catch {
747+
if (cd1 != (iconv_t)NULL) {
748+
iconv_close(cd1);
749+
}
734750
if (cd2 != (iconv_t)NULL) {
735-
_php_iconv_appendl(pretval, NULL, 0, cd2);
751+
iconv_close(cd2);
736752
}
737-
smart_str_0(pretval);
738-
}
753+
zend_bailout();
754+
} zend_end_try();
739755

740756
if (cd1 != (iconv_t)NULL) {
741757
iconv_close(cd1);
@@ -942,6 +958,8 @@ static php_iconv_err_t _php_iconv_mime_encode(smart_str *pretval, const char *fn
942958
goto out;
943959
}
944960

961+
buf = safe_emalloc(1, max_line_len, 5);
962+
945963
cd_pl = iconv_open(ICONV_ASCII_ENCODING, enc);
946964
if (cd_pl == (iconv_t)(-1)) {
947965
if (errno == EINVAL) {
@@ -962,10 +980,10 @@ static php_iconv_err_t _php_iconv_mime_encode(smart_str *pretval, const char *fn
962980
goto out;
963981
}
964982

965-
buf = safe_emalloc(1, max_line_len, 5);
966-
967983
char_cnt = max_line_len;
968984

985+
zend_try {
986+
969987
_php_iconv_appendl(pretval, fname, fname_nbytes, cd_pl);
970988
char_cnt -= fname_nbytes;
971989
smart_str_appendl(pretval, ": ", sizeof(": ") - 1);
@@ -1172,6 +1190,22 @@ static php_iconv_err_t _php_iconv_mime_encode(smart_str *pretval, const char *fn
11721190

11731191
smart_str_0(pretval);
11741192

1193+
} zend_catch {
1194+
if (cd != (iconv_t)(-1)) {
1195+
iconv_close(cd);
1196+
}
1197+
if (cd_pl != (iconv_t)(-1)) {
1198+
iconv_close(cd_pl);
1199+
}
1200+
if (encoded != NULL) {
1201+
zend_string_release_ex(encoded, 0);
1202+
}
1203+
if (buf != NULL) {
1204+
efree(buf);
1205+
}
1206+
zend_bailout();
1207+
} zend_end_try();
1208+
11751209
out:
11761210
if (cd != (iconv_t)(-1)) {
11771211
iconv_close(cd);
@@ -1224,6 +1258,7 @@ static php_iconv_err_t _php_iconv_mime_decode(smart_str *pretval, const char *st
12241258
}
12251259

12261260
p1 = str;
1261+
zend_try {
12271262
for (str_left = str_nbytes; str_left > 0; str_left--, p1++) {
12281263
int eos = 0;
12291264

@@ -1729,6 +1764,17 @@ static php_iconv_err_t _php_iconv_mime_decode(smart_str *pretval, const char *st
17291764
}
17301765

17311766
smart_str_0(pretval);
1767+
1768+
} zend_catch {
1769+
if (cd != (iconv_t)(-1)) {
1770+
iconv_close(cd);
1771+
}
1772+
if (cd_pl != (iconv_t)(-1)) {
1773+
iconv_close(cd_pl);
1774+
}
1775+
zend_bailout();
1776+
} zend_end_try();
1777+
17321778
out:
17331779
if (cd != (iconv_t)(-1)) {
17341780
iconv_close(cd);

‎ext/iconv/tests/gh17399.phpt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
--TEST--
2+
GH-17399 (iconv memory leak with large line-length in iconv_mime_encode)
3+
--EXTENSIONS--
4+
iconv
5+
--FILE--
6+
<?php
7+
$options = array(
8+
'line-length' => PHP_INT_MAX,
9+
);
10+
iconv_mime_encode('Subject', 'test', $options);
11+
?>
12+
--EXPECTF--
13+
Fatal error: Allowed memory size of %d bytes exhausted %s in %s on line %d

0 commit comments

Comments
 (0)