php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #71753 Multiple (possible) mistakes in PHP "standard" extension code
Submitted: 2016-03-09 12:08 UTC Modified: 2016-03-11 13:34 UTC
From: temp at temp dot ru Assigned: ab (profile)
Status: Closed Package: *General Issues
PHP Version: 7.0.4 OS:
Private report: No CVE-ID: None
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: temp at temp dot ru
New email:
PHP Version: OS:

 

 [2016-03-09 12:08 UTC] temp at temp dot ru
Description:
------------
There are several possible mistakes in "standard" PHP extension code.

1. flock_compat.c, php_flock function:

HANDLE hdl = (HANDLE) _get_osfhandle(fd);
DWORD low = 1, high = 0;
OVERLAPPED offset =
{0, 0, 0, 0, NULL};
DWORD err;

if (hdl < 0) { // <-- Comparing pointer < 0. Pointer value can't be less than zero. Correct comparison: hdl == INVALID_HANDLE_VALUE
	_set_errno(EBADF);
	return -1;              /* error in file descriptor */
}




2. filters.c, php_conv_qprint_encode_convert function, possible zero pointer dereferencing:

c = NEXT_CHAR(ps, icnt, lb_ptr, lb_cnt, inst->lbchars); // <-- Access to inst->lbchars[lb_ptr] inside the macro

if (!(opts & PHP_CONV_QPRINT_OPT_BINARY) &&
	(trail_ws == 0) &&
	(c == '\t' || c == ' ')) {
	if (line_ccnt < 2 && inst->lbchars != NULL) { // <-- Later check if inst->lbchars is not NULL



3. http_fopen_wrapper.c, php_stream_url_wrap_http_ex function, possible zero pointer dereferencing:

ua = emalloc(ua_len + 1);
if ((ua_len = slprintf(ua, ua_len, _UA_HEADER, ua_str)) > 0) { // <-- Access to ua
	ua[ua_len] = 0;
	php_stream_write(stream, ua, ua_len);
} else {
	php_error_docref(NULL, E_WARNING, "Cannot construct User-agent header");
}

if (ua) { // <-- Later check if ua is not NULL
	efree(ua);
}




4. uuencode.c, php_uudecode function:

assert(p >= ZSTR_VAL(dest));
if ((len = total_len > (size_t)(p - ZSTR_VAL(dest)))) { // <-- Possible absence of brackets: this expression should be if(((len = total_len) > ...))




5. var.c, php_var_serialize_intern function:

zend_class_entry *ce = Z_OBJCE_P(struc);

if (ce->serialize != NULL) { // <-- Access to ce->serialize

...

if (ce && ce != PHP_IC_ENTRY && zend_hash_str_exists(&ce->function_table, "__sleep", sizeof("__sleep")-1)) { // <-- Later check if ce is not NULL





6. var.c, php_var_serialize_intern function (again):

if (incomplete_class && strcmp(ZSTR_VAL(key), MAGIC_MEMBER) == 0) { // <-- Access to key->val
	continue;
}

if (!key) { // <-- Later check if key is not NULL
	php_var_serialize_long(buf, index);
} else {
	php_var_serialize_string(buf, ZSTR_VAL(key), ZSTR_LEN(key));
}


Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-03-09 12:28 UTC] temp at temp dot ru
Also, there's similar possible bug in SPL:

spl_dllist.c, spl_ptr_llist_destroy function:

while (current) {
	next = current->next; // --> Access to current->next
	if(current && dtor) { // --> Later check if current is not null
		dtor(current);
	}
 [2016-03-10 07:31 UTC] laruence@php.net
Hey:

    1. it's not wrong, at least INVALID_HANDLE_VALUE === -1
    2. not sure about this one.
    3. fixed
    4. fixed
    5. fixed
    6. not a problem, incomplete_class makes sure key is valid.
 [2016-03-10 07:53 UTC] temp at temp dot ru
The first one IS wrong, actually.

Check this: http://codepad.org/Q0edKW9A
 [2016-03-10 07:58 UTC] temp at temp dot ru
And please also take a look at the first comment with similar SPL code issue.
 [2016-03-10 08:03 UTC] laruence@php.net
-Assigned To: +Assigned To: ab
 [2016-03-10 08:03 UTC] laruence@php.net
hmm, so HANDLE is defined as unsigned...

@welting what do you think? (I am not windows dever, so...)
 [2016-03-10 08:04 UTC] laruence@php.net
yeah, spl one is also fixed, thanks for the reporting, your help is appreciated  :)
 [2016-03-11 13:34 UTC] ab@php.net
-Status: Assigned +Status: Closed
 [2016-03-11 13:34 UTC] ab@php.net
Yeah, thanks for the proposals. The HANDLE issue is fixed now. That code was about 20 years old :)

Thanks.
 
PHP Copyright © 2001-2025 The PHP Group
All rights reserved.
Last updated: Sat Sep 20 13:00:01 2025 UTC