Commits

Nick Wellnhofer committed f0c0e13

Fix double free when calling $node->addSibling with text nodes

In libxml2’s tree.c xmlAddSibling has an optimization where if the
existing node is text and you appendSibling a text node it will instead
append the text onto the existing sibling then xmlFree the new sibling.

Handle this special case by copying the text node before adding it.

Reported in RT #94149
https://rt.cpan.org/Ticket/Display.html?id=94149

Thanks to Jeff Trout for the report.

  • Participants
  • Parent commits 02fb4ba
  • Branches rt94149

Comments (0)

Files changed (3)

 Revision history for Perl extension XML::LibXML
 
 2.0114
+    - Fix double free when calling $node->addSibling with text nodes.
+        - https://rt.cpan.org/Ticket/Display.html?id=94149
+        - Thanks to Jeff Trout for the report.
     - Fix memory leaks and segfaults related to removal and insertion of
       DTD nodes.
         - https://rt.cpan.org/Ticket/Display.html?id=80521
         }
         owner = PmmOWNERPO(PmmPROXYNODE(self));
 
-        ret = xmlAddSibling( self, nNode );
-
-        if ( ret ) {
-            RETVAL = PmmNodeToSv(ret,NULL);
-            if (nNode->type == XML_DTD_NODE) {
-                LibXML_set_int_subset(self->doc, nNode);
-            }
-            PmmFixOwner(SvPROXYNODE(RETVAL), owner);
+        if (self->type == XML_TEXT_NODE && nNode->type == XML_TEXT_NODE
+            && self->name == nNode->name) {
+            /* As a result of text merging, the added node may be freed. */
+            xmlNodePtr copy = xmlCopyNode(nNode, 0);
+            ret = xmlAddSibling(self, copy);
+
+            if (ret) {
+                RETVAL = PmmNodeToSv(ret, owner);
+                /* Unlink original node. */
+                xmlUnlinkNode(nNode);
+                LibXML_reparent_removed_node(nNode);
+            }
+            else {
+                xmlFreeNode(copy);
+                XSRETURN_UNDEF;
+            }
         }
         else {
-            XSRETURN_UNDEF;
+            ret = xmlAddSibling( self, nNode );
+
+            if ( ret ) {
+                RETVAL = PmmNodeToSv(ret, owner);
+                if (nNode->type == XML_DTD_NODE) {
+                    LibXML_set_int_subset(self->doc, nNode);
+                }
+                PmmFixOwner(SvPROXYNODE(RETVAL), owner);
+            }
+            else {
+                XSRETURN_UNDEF;
+            }
         }
     OUTPUT:
         RETVAL
 # since all tests are run on a preparsed
 
 # Should be 166.
-use Test::More tests => 194;
+use Test::More tests => 195;
 
 use XML::LibXML;
 use XML::LibXML::Common qw(:libxml);
     }
 }
 
+{
+    # RT #94149
+    # https://rt.cpan.org/Ticket/Display.html?id=94149
+
+    my $orig = XML::LibXML::Text->new('Double ');
+    my $ret = $orig->addSibling(XML::LibXML::Text->new('Free'));
+    # TEST
+    is( $ret->textContent, 'Double Free', 'merge text nodes with addSibling' );
+}
+