Skip to content

Commit

Permalink
Properly fix #80220
Browse files Browse the repository at this point in the history
The original fix for that bug[1] broke the formerly working composition
of message/rfc822 messages, which results in a segfault when freeing
the message body now.  While `imap_mail_compose()` does not really
support composition of meaningful message/rfc822 messages (although
libc-client appears to support that), some code may still use this to
compose partial messages, and using string manipulation to create the
final message.

The point is that libc-client expects `TYPEMESSAGE` with an explicit
subtype of `RFC822` to have a `nested.msg` (otherwise there will be a
segfault during free), but not to have any `contents.text.data` (this
will leak otherwise).

[1] <http://git.php.net/?p=php-src.git;a=commit;h=0d022ddf03c5fabaaa22e486d1e4a367ed9170a7>

Closes GH-6343.
  • Loading branch information
cmb69 committed Oct 20, 2020
1 parent 7b5f232 commit 7f3bdda
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 9 deletions.
1 change: 1 addition & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ PHP NEWS
- IMAP:
. Fixed bug #64076 (imap_sort() does not return FALSE on failure). (cmb)
. Fixed bug #80239 (imap_rfc822_write_address() leaks memory). (cmb)
. Fixed minor regression caused by fixing bug #80220. (cmb)

29 Oct 2020, PHP 7.3.24

Expand Down
22 changes: 13 additions & 9 deletions ext/imap/php_imap.c
Original file line number Diff line number Diff line change
Expand Up @@ -3706,15 +3706,19 @@ PHP_FUNCTION(imap_mail_compose)
bod->disposition.parameter = disp_param;
}
}
if ((pvalue = zend_hash_str_find(Z_ARRVAL_P(data), "contents.data", sizeof("contents.data") - 1)) != NULL) {
convert_to_string_ex(pvalue);
bod->contents.text.data = fs_get(Z_STRLEN_P(pvalue) + 1);
memcpy(bod->contents.text.data, Z_STRVAL_P(pvalue), Z_STRLEN_P(pvalue)+1);
bod->contents.text.size = Z_STRLEN_P(pvalue);
if (bod->type == TYPEMESSAGE && bod->subtype && !strcmp(bod->subtype, "RFC822")) {
bod->nested.msg = mail_newmsg();
} else {
bod->contents.text.data = fs_get(1);
memcpy(bod->contents.text.data, "", 1);
bod->contents.text.size = 0;
if ((pvalue = zend_hash_str_find(Z_ARRVAL_P(data), "contents.data", sizeof("contents.data") - 1)) != NULL) {
convert_to_string_ex(pvalue);
bod->contents.text.data = fs_get(Z_STRLEN_P(pvalue) + 1);
memcpy(bod->contents.text.data, Z_STRVAL_P(pvalue), Z_STRLEN_P(pvalue)+1);
bod->contents.text.size = Z_STRLEN_P(pvalue);
} else {
bod->contents.text.data = fs_get(1);
memcpy(bod->contents.text.data, "", 1);
bod->contents.text.size = 0;
}
}
if ((pvalue = zend_hash_str_find(Z_ARRVAL_P(data), "lines", sizeof("lines") - 1)) != NULL) {
bod->size.lines = zval_get_long(pvalue);
Expand Down Expand Up @@ -3933,7 +3937,7 @@ PHP_FUNCTION(imap_mail_compose)
efree(mystring);
mystring=tempstring;
} else if (bod) {
spprintf(&tempstring, 0, "%s%s%s", mystring, bod->contents.text.data, CRLF);
spprintf(&tempstring, 0, "%s%s%s", mystring, bod->contents.text.data ? bod->contents.text.data : "", CRLF);
efree(mystring);
mystring=tempstring;
} else {
Expand Down
34 changes: 34 additions & 0 deletions ext/imap/tests/bug80220.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
--TEST--
Bug #80220 (imap_mail_compose() may leak memory) - message/rfc822 regression
--SKIPIF--
<?php
if (!extension_loaded('imap')) die('skip imap extension not available');
?>
--FILE--
<?php
$bodies = [[
'type' => TYPEMESSAGE,
'subtype' => 'RFC822',
], [
'contents.data' => 'asd',
]];
var_dump(imap_mail_compose([], $bodies));

$bodies = [[
'type' => TYPEMESSAGE,
], [
'contents.data' => 'asd',
]];
var_dump(imap_mail_compose([], $bodies));
?>
--EXPECT--
string(53) "MIME-Version: 1.0
Content-Type: MESSAGE/RFC822


"
string(53) "MIME-Version: 1.0
Content-Type: MESSAGE/RFC822


"

0 comments on commit 7f3bdda

Please sign in to comment.