Bug #71753 [Opn]: Multiple (possible) mistakes in PHP "standard" extension code

Edit report at https://bugs.php.net/bug.php?id=71753&edit=1 ID: 71753 Updated by: laruence@php.net Reported by: temp at temp dot ru Summary: Multiple (possible) mistakes in PHP "standard" extension code Status: Open Type: Bug Package: *General Issues PHP Version: 7.0.4 -Assigned To: +Assigned To: ab Block user comment: N Private report: N New Comment: hmm, so HANDLE is defined as unsigned... @welting what do you think? (I am not windows dever, so...) Previous Comments: ------------------------------------------------------------------------ [2016-03-10 07:58:29] temp at temp dot ru And please also take a look at the first comment with similar SPL code issue. ------------------------------------------------------------------------ [2016-03-10 07:53:16] temp at temp dot ru The first one IS wrong, actually. Check this: http://codepad.org/Q0edKW9A ------------------------------------------------------------------------ [2016-03-10 07:31:21] 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-09 12:28:26] 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-09 12:08:56] 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)); } ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=71753&edit=1