use-of-uninitialized-value in ext/dom/document.c:1406

YuanchengJiang commented 1 week ago

Description

The following code:

<?php
$dom = Dom\XMLDocument::createFromString(<<<XML
</root>
XML);

Resulted in this output:

==3299275==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x558b84f0d4bb in dom_document_parser /php-src/ext/dom/document.c:1406:7
    #1 0x558b851d5a62 in load_from_helper /php-src/ext/dom/xml_document.c:181:23
    #2 0x558b851d419e in zim_Dom_XMLDocument_createFromString /php-src/ext/dom/xml_document.c:248:2
    #3 0x558b89a58f3e in ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER /php-src/Zend/zend_vm_execute.h:2037:4
    #4 0x558b89760c8c in execute_ex /php-src/Zend/zend_vm_execute.h:58565:7
    #5 0x558b89763100 in zend_execute /php-src/Zend/zend_vm_execute.h:64217:2
    #6 0x558b8a6c7eb1 in zend_execute_script /php-src/Zend/zend.c:1928:3
    #7 0x558b88a99878 in php_execute_script_ex /php-src/main/main.c:2574:13
    #8 0x558b88a9af05 in php_execute_script /php-src/main/main.c:2614:9
    #9 0x558b8a6e6e72 in do_cli /php-src/sapi/cli/php_cli.c:935:5
    #10 0x558b8a6df813 in main /php-src/sapi/cli/php_cli.c:1310:18
    #11 0x7f45cac21d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #12 0x7f45cac21e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #13 0x558b84204e64 in _start (/php-src/sapi/cli/php+0x404e64) (BuildId: c072ee43b7ede295b7c6cf26ac9578686c8b04c4)

  Uninitialized value was created by a heap allocation
    #0 0x558b84238f10 in malloc (/php-src/sapi/cli/php+0x438f10) (BuildId: c072ee43b7ede295b7c6cf26ac9578686c8b04c4)
    #1 0x7f45cd231153 in xmlNewParserCtxt (/lib/x86_64-linux-gnu/libxml2.so.2+0x48153) (BuildId: aebf8e42966c3ce475ff9d9d51a762831adcbb61)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /php-src/ext/dom/document.c:1406:7

Wondering if we care about MSan errors?

PHP Version

PHP 8.4.0-dev

Operating System

ubuntu 22.04

devnexen commented 1 week ago

~~can't reproduce~~ (finally can when having it) and do not really get it, the pointed function memset the whole thing

// xml2 2.9.13
xmlParserCtxtPtr
xmlNewParserCtxt(void)
{
    xmlParserCtxtPtr ctxt;

    ctxt = (xmlParserCtxtPtr) xmlMalloc(sizeof(xmlParserCtxt));
    if (ctxt == NULL) {
    xmlErrMemory(NULL, "cannot allocate parser context\n");
    return(NULL);
    }
    memset(ctxt, 0, sizeof(xmlParserCtxt));
    if (xmlInitParserCtxt(ctxt) < 0) {
        xmlFreeParserCtxt(ctxt);
    return(NULL);
    }
    return(ctxt);
}

but maybe I misread.

devnexen commented 1 week ago

Wondering if we care about MSan errors?

We do to some extents, some are genuine but we do get more false positives than with other sanitizers.

nielsdos commented 1 week ago

I can check this tonight. @YuanchengJiang this may be a false positive. MSAN instruments code, but if you use the system libraries the instrumentation is missing for those libraries. Checking for uninit memory is better done with Valgrind or by recompiling the libraries under MSAN.

YuanchengJiang commented 1 week ago

I can check this tonight. @YuanchengJiang this may be a false positive. MSAN instruments code, but if you use the system libraries the instrumentation is missing for those libraries. Checking for uninit memory is better done with Valgrind or by recompiling the libraries under MSAN.

I see. Thanks

nielsdos commented 1 week ago

This is a false positive, testing on Valgrind or libxml+MSAN reports no issue. Also looking at the code it seems the memory is properly initialized. Closing as invalid.

php / php-src