php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #73162 missing NULL check in dom_document_savexml
Submitted: 2016-09-24 10:18 UTC Modified: 2016-10-19 14:25 UTC
From: nguyenluan dot vnn at gmail dot com Assigned:
Status: Duplicate Package: DOM XML related
PHP Version: 5.6.26 OS:
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: nguyenluan dot vnn at gmail dot com
New email:
PHP Version: OS:

 

 [2016-09-24 10:18 UTC] nguyenluan dot vnn at gmail dot com
Description:
------------
In dom_document_savexml function:

        } else {
		if (options & LIBXML_SAVE_NOEMPTYTAG) {
			saveempty = xmlSaveNoEmptyTags;
			xmlSaveNoEmptyTags = 1;
		}
		/* Encoding is handled from the encoding property set on the document */
		xmlDocDumpFormatMemory(docp, &mem, &size, format);
		if (options & LIBXML_SAVE_NOEMPTYTAG) {
			xmlSaveNoEmptyTags = saveempty;
		}
		if (!size) {
			RETURN_FALSE;
		}
		RETVAL_STRINGL(mem, size, 1);
		xmlFree(mem);
	}

If size != 0, a string will be retured. When running the PHP test script below, variable "size" is larger than INTMAX while variable "mem" in RETVAL_STRINGL(mem, size, 1) is NULL which leads to a memory access violation when RETVAL_STRINGL calls to memcpy later.

If *mem is not NULL, a string with size > 2GB is returned. This is also considered as security issue in PHP.

Test script:
---------------
<?php
    ini_set('memory_limit', -1);
    $str = str_repeat('a', 0x5fffffff);
    
    $doc = new DOMDocument('1.0');
    
    $doc->formatOutput = True;
    
    $root = $doc->createElement("test1");
    $root = $doc->appendChild($root);
    
    $text1 = $doc->createTextNode($str);
    $text1 = $root->appendChild($text1);
    
    $root = $doc->createElement("test2");
    $root = $doc->appendChild($root);
    
    $text2 = $doc->createTextNode($str);
    $text2 = $root->appendChild($text2);
    
    $str1 = $doc->saveXML();
    
    var_dump(strlen($str));
    var_dump(strlen($str1));
?>

Expected result:
----------------
No crash, no string return since XML string input is larger than 2Gb.

Actual result:
--------------
gdb-peda$ b document.c:1798
Breakpoint 1 at 0x618090: file /home/user/Desktop/php-5.6.26/ext/dom/document.c, line 1798.


gdb-peda$ r ../test/string/test_domxml.php 
Starting program: /home/user/Desktop/php-5.6.26/sapi/cli/php ../test/string/test_domxml.php
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
 [----------------------------------registers-----------------------------------]
RAX: 0x80000008 
RBX: 0xa93d73 (<execute_ex>:	push   rbp)
RCX: 0x7ffff4422e00 (<__mmap+64>:	ja     0x7ffff4422e58 <__mmap+152>)
RDX: 0x0 
RSI: 0x7ffff46e5b38 --> 0x1694b50 --> 0x0 
RDI: 0xffffffff 
RBP: 0x7fffffffa790 --> 0x7fffffffa800 --> 0x7fffffffa820 --> 0x7fffffffa850 --> 0x7fffffffa880 --> 0x7fffffffa9c0 (--> ...)
RSP: 0x7fffffffa6c0 --> 0x7ffff7fbd5b0 --> 0x5a5a5a5a00000001 
RIP: 0x618090 (<zif_dom_document_savexml+1082>:	mov    rax,QWORD PTR [rbp-0x88])
R8 : 0x1694b60 --> 0x0 
R9 : 0x60000000 ('')
R10: 0x1 
R11: 0x207 
R12: 0x439790 (<_start>:	xor    ebp,ebp)
R13: 0x7fffffffe1a0 --> 0x2 
R14: 0x0 
R15: 0x0
EFLAGS: 0x282 (carry parity adjust zero SIGN trap INTERRUPT direction overflow)
[-------------------------------------code-------------------------------------]
   0x618083 <zif_dom_document_savexml+1069>:	mov    rax,QWORD PTR [rbp-0x10]
   0x618087 <zif_dom_document_savexml+1073>:	mov    BYTE PTR [rax+0x14],0x3
   0x61808b <zif_dom_document_savexml+1077>:	
    jmp    0x618114 <zif_dom_document_savexml+1214>
=> 0x618090 <zif_dom_document_savexml+1082>:	mov    rax,QWORD PTR [rbp-0x88]
   0x618097 <zif_dom_document_savexml+1089>:	mov    QWORD PTR [rbp-0x20],rax
   0x61809b <zif_dom_document_savexml+1093>:	mov    eax,DWORD PTR [rbp-0xa8]
   0x6180a1 <zif_dom_document_savexml+1099>:	mov    DWORD PTR [rbp-0x9c],eax
   0x6180a7 <zif_dom_document_savexml+1105>:	mov    rax,QWORD PTR [rbp-0xc0]
[------------------------------------stack-------------------------------------]
0000| 0x7fffffffa6c0 --> 0x7ffff7fbd5b0 --> 0x5a5a5a5a00000001 
0008| 0x7fffffffa6c8 --> 0x7ffff7f86290 --> 0x7ffff7fc1638 ('Z' <repeats 16 times>, "\001")
0016| 0x7fffffffa6d0 --> 0x7ffff7fc1638 ('Z' <repeats 16 times>, "\001")
0024| 0x7fffffffa6d8 --> 0x1 
0032| 0x7fffffffa6e0 --> 0x7ffff7fc15e8 --> 0x7fffae383fab ('a' <repeats 200 times>...)
0040| 0x7fffffffa6e8 --> 0x80000008 
0048| 0x7fffffffa6f0 --> 0x7fff00000001 ('a' <repeats 200 times>...)
0056| 0x7fffffffa6f8 --> 0x7ffff7fbd5b0 --> 0x5a5a5a5a00000001 
[------------------------------------------------------------------------------]
Legend: code, data, rodata, value

Breakpoint 1, zif_dom_document_savexml (ht=0x0, return_value=0x7ffff7fc1638, 
    return_value_ptr=0x7ffff7f86290, this_ptr=0x7ffff7fbd5b0, 
    return_value_used=0x1)
    at /home/user/Desktop/php-5.6.26/ext/dom/document.c:1798
1798			RETVAL_STRINGL(mem, size, 1);


gdb-peda$ p size
$1 = 0x80000008       <- size is larger than INTMAX


gdb-peda$ p mem
$2 = (xmlChar *) 0x0  <- mem is NULL


gdb-peda$ b zend_alloc.c:2664
Breakpoint 2 at 0xa110da: file /home/user/Desktop/php-5.6.26/Zend/zend_alloc.c, line 2664.


gdb-peda$ c
Continuing.

 [----------------------------------registers-----------------------------------]
RAX: 0x0 
RBX: 0xa93d73 (<execute_ex>:	push   rbp)
RCX: 0x7ffe4d12e010 --> 0x80040000 
RDX: 0xe003d8c0 
RSI: 0x7ffecd12e080 --> 0x7ffe94d6cac3 --> 0x0 
RDI: 0x1425700 --> 0x1 
RBP: 0x7fffffffa6b0 --> 0x7fffffffa790 --> 0x7fffffffa800 --> 0x7fffffffa820 --> 0x7fffffffa850 --> 0x7fffffffa880 (--> ...)
RSP: 0x7fffffffa670 --> 0x7fffffffa6e8 --> 0x80000008 
RIP: 0xa110da (<_estrndup+159>:	mov    edx,DWORD PTR [rbp-0x1c])
R8 : 0xffffffffffffffff 
R9 : 0x0 
R10: 0x22 ('"')
R11: 0x246 
R12: 0x439790 (<_start>:	xor    ebp,ebp)
R13: 0x7fffffffe1a0 --> 0x2 
R14: 0x0 
R15: 0x0
EFLAGS: 0x246 (carry PARITY adjust ZERO sign trap INTERRUPT direction overflow)
[-------------------------------------code-------------------------------------]
   0xa110d2 <_estrndup+151>:	call   rax
   0xa110d4 <_estrndup+153>:	mov    rax,QWORD PTR [rbp-0x8]
   0xa110d8 <_estrndup+157>:	jmp    0xa1111c <_estrndup+225>
=> 0xa110da <_estrndup+159>:	mov    edx,DWORD PTR [rbp-0x1c]
   0xa110dd <_estrndup+162>:	mov    rcx,QWORD PTR [rbp-0x18]
   0xa110e1 <_estrndup+166>:	mov    rax,QWORD PTR [rbp-0x8]
   0xa110e5 <_estrndup+170>:	mov    rsi,rcx
   0xa110e8 <_estrndup+173>:	mov    rdi,rax
[------------------------------------stack-------------------------------------]
0000| 0x7fffffffa670 --> 0x7fffffffa6e8 --> 0x80000008 
0008| 0x7fffffffa678 --> 0x1aacad00 
0016| 0x7fffffffa680 --> 0x0 
0024| 0x7fffffffa688 --> 0xc14c48 ("/home/user/Desktop/php-5.6.26/ext/dom/document.c")
0032| 0x7fffffffa690 --> 0x8000000800000706 
0040| 0x7fffffffa698 --> 0x0 
0048| 0x7fffffffa6a0 --> 0x7fffffffe1a0 --> 0x2 
0056| 0x7fffffffa6a8 --> 0x7ffe4d12e070 --> 0x0 
[------------------------------------------------------------------------------]
Legend: code, data, rodata, value

Breakpoint 2, _estrndup (s=0x0, length=0x80000008, 
    __zend_filename=0xc14c48 "/home/user/Desktop/php-5.6.26/ext/dom/document.c", __zend_lineno=0x706, __zend_orig_filename=0x0, __zend_orig_lineno=0x0)
    at /home/user/Desktop/php-5.6.26/Zend/zend_alloc.c:2664
2664		memcpy(p, s, length);


gdb-peda$ p length
$3 = 0x80000008 <- length is larger than INTMAX


gdb-peda$ p s
$4 = 0x0        <- source string is NULL


gdb-peda$ p p
$5 = 0x7ffe4d12e070 ""
gdb-peda$ n

Program received signal SIGSEGV, Segmentation fault.

 [----------------------------------registers-----------------------------------]
RAX: 0xffff800132ed1f88 
RBX: 0xa93d73 (<execute_ex>:	push   rbp)
RCX: 0x100000010 
RDX: 0x80000008 
RSI: 0x0 
RDI: 0x7ffe4d12e070 --> 0x0 
RBP: 0x7fffffffa6b0 --> 0x7fffffffa790 --> 0x7fffffffa800 --> 0x7fffffffa820 --> 0x7fffffffa850 --> 0x7fffffffa880 (--> ...)
RSP: 0x7fffffffa668 --> 0xa110f0 (<_estrndup+181>:	mov    edx,DWORD PTR [rbp-0x1c])
RIP: 0x7ffff43c0e10 (<__memcpy_sse2_unaligned+32>:	movdqu xmm8,XMMWORD PTR [rsi])
R8 : 0xffffffffffffffff 
R9 : 0x0 
R10: 0x22 ('"')
R11: 0x246 
R12: 0x439790 (<_start>:	xor    ebp,ebp)
R13: 0x7fffffffe1a0 --> 0x2 
R14: 0x0 
R15: 0x0
EFLAGS: 0x10202 (carry parity adjust zero sign trap INTERRUPT direction overflow)
[-------------------------------------code-------------------------------------]
   0x7ffff43c0e00 <__memcpy_sse2_unaligned+16>:	
    jb     0x7ffff43c0f0d <__memcpy_sse2_unaligned+285>
   0x7ffff43c0e06 <__memcpy_sse2_unaligned+22>:	cmp    rdx,0x10
   0x7ffff43c0e0a <__memcpy_sse2_unaligned+26>:	
    jbe    0x7ffff43c0f9b <__memcpy_sse2_unaligned+427>
=> 0x7ffff43c0e10 <__memcpy_sse2_unaligned+32>:	movdqu xmm8,XMMWORD PTR [rsi]
   0x7ffff43c0e15 <__memcpy_sse2_unaligned+37>:	cmp    rdx,0x20
   0x7ffff43c0e19 <__memcpy_sse2_unaligned+41>:	movdqu XMMWORD PTR [rdi],xmm8
   0x7ffff43c0e1e <__memcpy_sse2_unaligned+46>:	
    movdqu xmm8,XMMWORD PTR [rsi+rdx*1-0x10]
   0x7ffff43c0e25 <__memcpy_sse2_unaligned+53>:	
    movdqu XMMWORD PTR [rdi+rdx*1-0x10],xmm8
[------------------------------------stack-------------------------------------]
0000| 0x7fffffffa668 --> 0xa110f0 (<_estrndup+181>:	mov    edx,DWORD PTR [rbp-0x1c])
0008| 0x7fffffffa670 --> 0x7fffffffa6e8 --> 0x80000008 
0016| 0x7fffffffa678 --> 0x1aacad00 
0024| 0x7fffffffa680 --> 0x0 
0032| 0x7fffffffa688 --> 0xc14c48 ("/home/user/Desktop/php-5.6.26/ext/dom/document.c")
0040| 0x7fffffffa690 --> 0x8000000800000706 
0048| 0x7fffffffa698 --> 0x0 
0056| 0x7fffffffa6a0 --> 0x7fffffffe1a0 --> 0x2 
[------------------------------------------------------------------------------]
Legend: code, data, rodata, value
Stopped reason: SIGSEGV
__memcpy_sse2_unaligned ()
    at ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S:35
35	../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S: No such file or directory.
gdb-peda$ bt
#0  __memcpy_sse2_unaligned ()
    at ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S:35
#1  0x0000000000a110f0 in _estrndup (s=0x0, length=0x80000008, 
    __zend_filename=0xc14c48 "/home/user/Desktop/php-5.6.26/ext/dom/document.c", __zend_lineno=0x706, __zend_orig_filename=0x0, __zend_orig_lineno=0x0)
    at /home/user/Desktop/php-5.6.26/Zend/zend_alloc.c:2664
#2  0x00000000006180e9 in zif_dom_document_savexml (ht=0x0, 
    return_value=0x7ffff7fc1638, return_value_ptr=0x7ffff7f86290, 
    this_ptr=0x7ffff7fbd5b0, return_value_used=0x1)
    at /home/user/Desktop/php-5.6.26/ext/dom/document.c:1798
#3  0x0000000000a9476b in zend_do_fcall_common_helper_SPEC (
    execute_data=0x7ffff7f866c8)
    at /home/user/Desktop/php-5.6.26/Zend/zend_vm_execute.h:558
#4  0x0000000000a94f3e in ZEND_DO_FCALL_BY_NAME_SPEC_HANDLER (
    execute_data=0x7ffff7f866c8)
    at /home/user/Desktop/php-5.6.26/Zend/zend_vm_execute.h:693
#5  0x0000000000a93dd3 in execute_ex (execute_data=0x7ffff7f866c8)
    at /home/user/Desktop/php-5.6.26/Zend/zend_vm_execute.h:363
#6  0x0000000000a93e5a in zend_execute (op_array=0x7ffff7fbe488)
    at /home/user/Desktop/php-5.6.26/Zend/zend_vm_execute.h:388
#7  0x0000000000a4c498 in zend_execute_scripts (type=0x8, retval=0x0, 
    file_count=0x3) at /home/user/Desktop/php-5.6.26/Zend/zend.c:1341
#8  0x00000000009ad757 in php_execute_script (primary_file=0x7fffffffcd70)
    at /home/user/Desktop/php-5.6.26/main/main.c:2613
#9  0x0000000000b09556 in do_cli (argc=0x2, argv=0x13f5560)
    at /home/user/Desktop/php-5.6.26/sapi/cli/php_cli.c:994
#10 0x0000000000b0a8b9 in main (argc=0x2, argv=0x13f5560)
    at /home/user/Desktop/php-5.6.26/sapi/cli/php_cli.c:1378
#11 0x00007ffff4342830 in __libc_start_main (main=0xb0a09c <main>, argc=0x2, 
    argv=0x7fffffffe1a8, init=<optimized out>, fini=<optimized out>, 
    rtld_fini=<optimized out>, stack_end=0x7fffffffe198)
    at ../csu/libc-start.c:291
#12 0x00000000004397b9 in _start ()


Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-09-25 23:15 UTC] stas@php.net
-Status: Open +Status: Feedback
 [2016-09-25 23:15 UTC] stas@php.net
I'm not sure, how it is a security issue? Please explain.
 [2016-09-26 12:34 UTC] nguyenluan dot vnn at gmail dot com
-Status: Feedback +Status: Open
 [2016-09-26 12:34 UTC] nguyenluan dot vnn at gmail dot com
This is NULL pointer dereference vulnerability. One more thing is even if "mem" is not NULL, this function could return a string with size larger than 2Gb, which is invalid string in PHP 5. A lot of string related functions in PHP 5 base on this assumption to declare string size as INT which could cause those functions vulnerable to stack buffer overflow or heap buffer overflow.

Like this one https://bugs.php.net/bug.php?id=73150, I think this issue should be consider as security issue.
 [2016-09-26 19:17 UTC] stas@php.net
-Status: Open +Status: Duplicate
 [2016-09-26 19:17 UTC] stas@php.net
libxml defines size as int in its API, so it's not supposed to return anything larger than can fit in int. If it does, please submit the bug to libxml maintainers, we can't really fix libxml. 

Also, now I see that it seems to be the same issue as #73162.
 [2016-09-26 19:17 UTC] stas@php.net
Oops, I means bug #73150.
 [2016-09-27 08:22 UTC] nguyenluan dot vnn at gmail dot com
I think it's not a duplicate since #73150 is missing null check in "dom_document_save_html" and #73162 (this bug) is missing null check in dom_document_savexml. They also use 2 different function from libxml2: "xmlDocDumpFormatMemory" and "htmlDocDumpMemoryFormat" or "htmlDocDumpMemory". Apply the patch for bug #73150 can't resolve bug #73162 (this bug).

I think libxml2 does it right when dealing with string larger than 2Gb because they return NULL "mem" but the problem is they also return the "size" value. PHP relies on this "size" value to return a string without checking "mem" is NULL or not. The cause the crash in RETVAL_STRINGL(mem, size, 1).

This is why I think PHP should add a check for "mem" and "size >= INTMAX" to prevent problems.
 [2016-10-19 14:19 UTC] nguyenluan dot vnn at gmail dot com
Sorry, I didn't notice you already fixed this issue in patch for bug #73150.
Please close this report since it's dupplicate.

Thanks.
 [2016-10-19 14:25 UTC] kaplan@php.net
Fix by 1c0e9126fbfb7fde3173347b7464237f56c38bfa (done for bug #73150)
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Mon Dec 30 14:01:28 2024 UTC