php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #78819 Heap Overflow in msg_send
Submitted: 2019-11-15 13:47 UTC Modified: 2019-11-19 06:09 UTC
From: jr at coredu dot mp Assigned:
Status: Open Package: Semaphore related
PHP Version: 7.3.11 OS: 64 bit Posix Systems
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: jr at coredu dot mp
New email:
PHP Version: OS:

 

 [2019-11-15 13:47 UTC] jr at coredu dot mp
Description:
------------
When sending a message that is 0x7FFFFFFF bytes large (maximum positive 32 bit integer), an integer overflow in msg_send causes a subsequent memcpy to overflow the messagebuffer.

ext/sysvmsg/sysvmsg.c:

PHP_FUNCTION(msg_send)
{
    ...
	int message_len = 0;
	
    if (do_serialize) {
	    ...
    } else {
		char *p;
		switch (Z_TYPE_P(message)) {
			case IS_STRING:
				p = Z_STRVAL_P(message);
				message_len = Z_STRLEN_P(message);
				break;

			case IS_LONG:
				message_len = spprintf(&p, 0, ZEND_LONG_FMT, Z_LVAL_P(message));
				break;
			case IS_FALSE:
				message_len = spprintf(&p, 0, "0");
				break;
			case IS_TRUE:
				message_len = spprintf(&p, 0, "1");
				break;
			case IS_DOUBLE:
				message_len = spprintf(&p, 0, "%F", Z_DVAL_P(message));
				break;
			default:
				php_error_docref(NULL, E_WARNING, "Message parameter must be either a string or a number.");
				RETURN_FALSE;
		}

		messagebuffer = safe_emalloc(message_len, 1, sizeof(struct php_msgbuf));
		memcpy(messagebuffer->mtext, p, message_len + 1);

		if (Z_TYPE_P(message) != IS_STRING) {
			efree(p);
		}
	}
	...
}

Since message_len is a signed 32 bit integer, the maximum positive value that it can hold is 0x7FFFFFFF.
When memcpy is called, 1 additional byte is added to message_len, this will overflow the integer and set the value to "-1". Memcpy actually expects an argument of type size_t which is an unsigned 64 bit integer.
This means the 32 bit -1 will be implicitly converted to a 64 bit -1 and then interpreted as an unsigned value which is much larger than the allocated buffer. This causes a heap overflow.

Test script:
---------------
<?php
ini_set("memory_limit", -1);

$a = msg_get_queue(234);
$a = msg_send($a, 1, str_repeat("a", 0x7FFFFFFF), false);
echo "$a\n"
?>


Expected result:
----------------
No Segmentation Fault

Actual result:
--------------
Segmentation Fault

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-11-17 11:37 UTC] cmb@php.net
ext/sysvmsg/sysvmsg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ext/sysvmsg/sysvmsg.c b/ext/sysvmsg/sysvmsg.c
index 6384ace349..a0e188d980 100644
--- a/ext/sysvmsg/sysvmsg.c
+++ b/ext/sysvmsg/sysvmsg.c
@@ -391,7 +391,7 @@ PHP_FUNCTION(msg_send)
 	sysvmsg_queue_t * mq = NULL;
 	struct php_msgbuf * messagebuffer = NULL; /* buffer to transmit */
 	int result;
-	int message_len = 0;
+	size_t message_len = 0;
 
 	RETVAL_FALSE;
 [2019-11-19 06:09 UTC] stas@php.net
-Type: Security +Type: Bug
 
PHP Copyright © 2001-2020 The PHP Group
All rights reserved.
Last updated: Mon Oct 26 02:01:23 2020 UTC