From b7d2ad6e1b376c10edffcb0973485c861dc89559 Mon Sep 17 00:00:00 2001 From: David Kilzer Date: Mon, 23 Jun 2025 14:41:56 -0700 Subject: [PATCH] libxslt: heap-use-after-free in xmlFreeID caused by `atype` corruption * include/libxml/tree.h: (XML_ATTR_CLEAR_ATYPE): Add. (XML_ATTR_GET_ATYPE): Add. (XML_ATTR_SET_ATYPE): Add. (XML_NODE_ADD_EXTRA): Add. (XML_NODE_CLEAR_EXTRA): Add. (XML_NODE_GET_EXTRA): Add. (XML_NODE_SET_EXTRA): Add. (XML_DOC_ADD_PROPERTIES): Add. (XML_DOC_CLEAR_PROPERTIES): Add. (XML_DOC_GET_PROPERTIES): Add. (XML_DOC_SET_PROPERTIES): Add. - Add macros for accessing fields with upper bits that may be set by libxslt. * HTMLparser.c: (htmlNewDocNoDtD): * SAX2.c: (xmlSAX2StartDocument): (xmlSAX2EndDocument): * parser.c: (xmlParseEntityDecl): (xmlParseExternalSubset): (xmlParseReference): (xmlCtxtParseDtd): * runxmlconf.c: (xmlconfTestInvalid): (xmlconfTestValid): * tree.c: (xmlNewDoc): (xmlFreeProp): (xmlNodeSetDoc): (xmlSetNsProp): (xmlDOMWrapAdoptBranch): * valid.c: (xmlFreeID): (xmlAddIDInternal): (xmlValidateAttributeValueInternal): (xmlValidateOneAttribute): (xmlValidateRef): * xmlreader.c: (xmlTextReaderStartElement): (xmlTextReaderStartElementNs): (xmlTextReaderValidateEntity): (xmlTextReaderRead): (xmlTextReaderNext): (xmlTextReaderIsEmptyElement): (xmlTextReaderPreserve): * xmlschemas.c: (xmlSchemaPValAttrNodeID): * xmlschemastypes.c: (xmlSchemaValAtomicType): - Adopt macros by renaming the struct fields, recompiling and fixing compiler failures, then changing the struct field names back. --- HTMLparser.c | 2 +- SAX2.c | 14 +++++++------- include/libxml/tree.h | 14 +++++++++++++- parser.c | 10 +++++----- runxmlconf.c | 4 ++-- tree.c | 14 +++++++------- valid.c | 14 +++++++------- xmlreader.c | 30 +++++++++++++++--------------- xmlschemas.c | 2 +- xmlschemastypes.c | 10 +++++----- 10 files changed, 63 insertions(+), 51 deletions(-) Index: libxml2-2.11.6/HTMLparser.c =================================================================== --- libxml2-2.11.6.orig/HTMLparser.c +++ libxml2-2.11.6/HTMLparser.c @@ -2489,7 +2489,7 @@ htmlNewDocNoDtD(const xmlChar *URI, cons cur->refs = NULL; cur->_private = NULL; cur->charset = XML_CHAR_ENCODING_UTF8; - cur->properties = XML_DOC_HTML | XML_DOC_USERBUILT; + XML_DOC_SET_PROPERTIES(cur, XML_DOC_HTML | XML_DOC_USERBUILT); if ((ExternalID != NULL) || (URI != NULL)) xmlCreateIntSubset(cur, BAD_CAST "html", ExternalID, URI); Index: libxml2-2.11.6/SAX2.c =================================================================== --- libxml2-2.11.6.orig/SAX2.c +++ libxml2-2.11.6/SAX2.c @@ -973,7 +973,7 @@ xmlSAX2StartDocument(void *ctx) xmlSAX2ErrMemory(ctxt, "xmlSAX2StartDocument"); return; } - ctxt->myDoc->properties = XML_DOC_HTML; + XML_DOC_SET_PROPERTIES(ctxt->myDoc, XML_DOC_HTML); ctxt->myDoc->parseFlags = ctxt->options; #else xmlGenericError(xmlGenericErrorContext, @@ -986,9 +986,9 @@ xmlSAX2StartDocument(void *ctx) } else { doc = ctxt->myDoc = xmlNewDoc(ctxt->version); if (doc != NULL) { - doc->properties = 0; + XML_DOC_CLEAR_PROPERTIES(doc); if (ctxt->options & XML_PARSE_OLD10) - doc->properties |= XML_DOC_OLD10; + XML_DOC_ADD_PROPERTIES(doc, XML_DOC_OLD10); doc->parseFlags = ctxt->options; if (ctxt->encoding != NULL) doc->encoding = xmlStrdup(ctxt->encoding); Index: libxml2-2.11.6/include/libxml/tree.h =================================================================== --- libxml2-2.11.6.orig/include/libxml/tree.h +++ libxml2-2.11.6/include/libxml/tree.h @@ -367,7 +367,6 @@ struct _xmlElement { #endif }; - /** * XML_LOCAL_NAMESPACE: * @@ -454,6 +453,10 @@ struct _xmlAttr { * An XML ID instance. */ +#define XML_ATTR_CLEAR_ATYPE(attr) (((attr)->atype) = 0) +#define XML_ATTR_GET_ATYPE(attr) (((attr)->atype) & ~(15U << 27)) +#define XML_ATTR_SET_ATYPE(attr, type) ((attr)->atype = ((((attr)->atype) & (15U << 27)) | ((type) & ~(15U << 27)))) + typedef struct _xmlID xmlID; typedef xmlID *xmlIDPtr; struct _xmlID { @@ -543,6 +546,11 @@ typedef enum { XML_DOC_HTML = 1<<7 /* parsed or built HTML document */ } xmlDocProperties; +#define XML_NODE_ADD_EXTRA(node, type) ((node)->extra |= ((type) & ~(15U << 12))) +#define XML_NODE_CLEAR_EXTRA(node) (((node)->extra) = 0) +#define XML_NODE_GET_EXTRA(node) (((node)->extra) & ~(15U << 12)) +#define XML_NODE_SET_EXTRA(node, type) ((node)->extra = ((((node)->extra) & (15U << 12)) | ((type) & ~(15U << 12)))) + /** * xmlDoc: * @@ -587,6 +595,10 @@ struct _xmlDoc { set at the end of parsing */ }; +#define XML_DOC_ADD_PROPERTIES(doc, type) ((doc)->properties |= ((type) & ~(15U << 27))) +#define XML_DOC_CLEAR_PROPERTIES(doc) (((doc)->properties) = 0) +#define XML_DOC_GET_PROPERTIES(doc) (((doc)->properties) & ~(15U << 27)) +#define XML_DOC_SET_PROPERTIES(doc, type) ((doc)->properties = ((((doc)->properties) & (15U << 27)) | ((type) & ~(15U << 27)))) typedef struct _xmlDOMWrapCtxt xmlDOMWrapCtxt; typedef xmlDOMWrapCtxt *xmlDOMWrapCtxtPtr; Index: libxml2-2.11.6/parser.c =================================================================== --- libxml2-2.11.6.orig/parser.c +++ libxml2-2.11.6/parser.c @@ -5491,7 +5491,7 @@ xmlParseEntityDecl(xmlParserCtxtPtr ctxt xmlErrMemory(ctxt, "New Doc failed"); goto done; } - ctxt->myDoc->properties = XML_DOC_INTERNAL; + XML_DOC_SET_PROPERTIES(ctxt->myDoc, XML_DOC_INTERNAL); } if (ctxt->myDoc->intSubset == NULL) ctxt->myDoc->intSubset = xmlNewDtd(ctxt->myDoc, @@ -5562,7 +5562,7 @@ xmlParseEntityDecl(xmlParserCtxtPtr ctxt xmlErrMemory(ctxt, "New Doc failed"); goto done; } - ctxt->myDoc->properties = XML_DOC_INTERNAL; + XML_DOC_SET_PROPERTIES(ctxt->myDoc, XML_DOC_INTERNAL); } if (ctxt->myDoc->intSubset == NULL) @@ -7024,7 +7024,7 @@ xmlParseExternalSubset(xmlParserCtxtPtr xmlErrMemory(ctxt, "New Doc failed"); return; } - ctxt->myDoc->properties = XML_DOC_INTERNAL; + XML_DOC_SET_PROPERTIES(ctxt->myDoc, XML_DOC_INTERNAL); } if ((ctxt->myDoc != NULL) && (ctxt->myDoc->intSubset == NULL)) xmlCreateIntSubset(ctxt->myDoc, NULL, ExternalID, SystemID); @@ -12384,7 +12384,7 @@ xmlIOParseDTD(xmlSAXHandlerPtr sax, xmlP xmlErrMemory(ctxt, "New Doc failed"); return(NULL); } - ctxt->myDoc->properties = XML_DOC_INTERNAL; + XML_DOC_SET_PROPERTIES(ctxt->myDoc, XML_DOC_INTERNAL); ctxt->myDoc->extSubset = xmlNewDtd(ctxt->myDoc, BAD_CAST "none", BAD_CAST "none", BAD_CAST "none"); Index: libxml2-2.11.6/runxmlconf.c =================================================================== --- libxml2-2.11.6.orig/runxmlconf.c +++ libxml2-2.11.6/runxmlconf.c @@ -192,7 +192,7 @@ xmlconfTestInvalid(const char *id, const id, filename); } else { /* invalidity should be reported both in the context and in the document */ - if ((ctxt->valid != 0) || (doc->properties & XML_DOC_DTDVALID)) { + if ((ctxt->valid != 0) || (XML_DOC_GET_PROPERTIES(doc) & XML_DOC_DTDVALID)) { test_log("test %s : %s failed to detect invalid document\n", id, filename); nb_errors++; @@ -224,7 +224,7 @@ xmlconfTestValid(const char *id, const c ret = 0; } else { /* validity should be reported both in the context and in the document */ - if ((ctxt->valid == 0) || ((doc->properties & XML_DOC_DTDVALID) == 0)) { + if ((ctxt->valid == 0) || ((XML_DOC_GET_PROPERTIES(doc) & XML_DOC_DTDVALID) == 0)) { test_log("test %s : %s failed to validate a valid document\n", id, filename); nb_errors++; Index: libxml2-2.11.6/tree.c =================================================================== --- libxml2-2.11.6.orig/tree.c +++ libxml2-2.11.6/tree.c @@ -1188,7 +1188,7 @@ xmlNewDoc(const xmlChar *version) { cur->compression = -1; /* not initialized */ cur->doc = cur; cur->parseFlags = 0; - cur->properties = XML_DOC_USERBUILT; + XML_DOC_SET_PROPERTIES(cur, XML_DOC_USERBUILT); /* * The in memory encoding is always UTF8 * This field will never change and would @@ -2125,7 +2125,7 @@ xmlFreeProp(xmlAttrPtr cur) { xmlDeregisterNodeDefaultValue((xmlNodePtr)cur); /* Check for ID removal -> leading to invalid references ! */ - if ((cur->doc != NULL) && (cur->atype == XML_ATTRIBUTE_ID)) { + if ((cur->doc != NULL) && (XML_ATTR_GET_ATYPE(cur) == XML_ATTRIBUTE_ID)) { xmlRemoveID(cur->doc, cur); } if (cur->children != NULL) xmlFreeNodeList(cur->children); @@ -7023,9 +7023,9 @@ xmlSetNsProp(xmlNodePtr node, xmlNsPtr n /* * Modify the attribute's value. */ - if (prop->atype == XML_ATTRIBUTE_ID) { + if (XML_ATTR_GET_ATYPE(prop) == XML_ATTRIBUTE_ID) { xmlRemoveID(node->doc, prop); - prop->atype = XML_ATTRIBUTE_ID; + XML_ATTR_SET_ATYPE(prop, XML_ATTRIBUTE_ID); } if (prop->children != NULL) xmlFreeNodeList(prop->children); @@ -7045,7 +7045,7 @@ xmlSetNsProp(xmlNodePtr node, xmlNsPtr n tmp = tmp->next; } } - if (prop->atype == XML_ATTRIBUTE_ID) + if (XML_ATTR_GET_ATYPE(prop) == XML_ATTRIBUTE_ID) xmlAddID(NULL, node->doc, value, prop); return(prop); } @@ -9300,7 +9300,7 @@ ns_end: if (cur->type == XML_ELEMENT_NODE) { cur->psvi = NULL; cur->line = 0; - cur->extra = 0; + XML_NODE_CLEAR_EXTRA(cur); /* * Walk attributes. */ Index: libxml2-2.11.6/valid.c =================================================================== --- libxml2-2.11.6.orig/valid.c +++ libxml2-2.11.6/valid.c @@ -3523,7 +3523,7 @@ xmlIsMixedElement(xmlDocPtr doc, const x static int xmlIsDocNameStartChar(xmlDocPtr doc, int c) { - if ((doc == NULL) || (doc->properties & XML_DOC_OLD10) == 0) { + if ((doc == NULL) || (XML_DOC_GET_PROPERTIES(doc) & XML_DOC_OLD10) == 0) { /* * Use the new checks of production [4] [4a] amd [5] of the * Update 5 of XML-1.0 @@ -3553,7 +3553,7 @@ xmlIsDocNameStartChar(xmlDocPtr doc, int static int xmlIsDocNameChar(xmlDocPtr doc, int c) { - if ((doc == NULL) || (doc->properties & XML_DOC_OLD10) == 0) { + if ((doc == NULL) || (XML_DOC_GET_PROPERTIES(doc) & XML_DOC_OLD10) == 0) { /* * Use the new checks of production [4] [4a] amd [5] of the * Update 5 of XML-1.0 @@ -4492,7 +4492,7 @@ xmlValidateOneAttribute(xmlValidCtxtPtr attr->name, elem->name, NULL); return(0); } - attr->atype = attrDecl->atype; + XML_ATTR_SET_ATYPE(attr, attrDecl->atype); val = xmlValidateAttributeValueInternal(doc, attrDecl->atype, value); if (val == 0) { @@ -6570,7 +6570,7 @@ xmlValidateRef(xmlRefPtr ref, xmlValidCt while (IS_BLANK_CH(*cur)) cur++; } xmlFree(dup); - } else if (attr->atype == XML_ATTRIBUTE_IDREF) { + } else if (XML_ATTR_GET_ATYPE(attr) == XML_ATTRIBUTE_IDREF) { id = xmlGetID(ctxt->doc, name); if (id == NULL) { xmlErrValidNode(ctxt, attr->parent, XML_DTD_UNKNOWN_ID, @@ -6578,7 +6578,7 @@ xmlValidateRef(xmlRefPtr ref, xmlValidCt attr->name, name, NULL); ctxt->valid = 0; } - } else if (attr->atype == XML_ATTRIBUTE_IDREFS) { + } else if (XML_ATTR_GET_ATYPE(attr) == XML_ATTRIBUTE_IDREFS) { xmlChar *dup, *str = NULL, *cur, save; dup = xmlStrdup(name); Index: libxml2-2.11.6/xmlreader.c =================================================================== --- libxml2-2.11.6.orig/xmlreader.c +++ libxml2-2.11.6/xmlreader.c @@ -610,7 +610,7 @@ xmlTextReaderStartElement(void *ctx, con if ((ctxt->node != NULL) && (ctxt->input != NULL) && (ctxt->input->cur != NULL) && (ctxt->input->cur[0] == '/') && (ctxt->input->cur[1] == '>')) - ctxt->node->extra = NODE_IS_EMPTY; + XML_NODE_SET_EXTRA(ctxt->node, NODE_IS_EMPTY); } if (reader != NULL) reader->state = XML_TEXTREADER_ELEMENT; @@ -675,7 +675,7 @@ xmlTextReaderStartElementNs(void *ctx, if ((ctxt->node != NULL) && (ctxt->input != NULL) && (ctxt->input->cur != NULL) && (ctxt->input->cur[0] == '/') && (ctxt->input->cur[1] == '>')) - ctxt->node->extra = NODE_IS_EMPTY; + XML_NODE_SET_EXTRA(ctxt->node, NODE_IS_EMPTY); } if (reader != NULL) reader->state = XML_TEXTREADER_ELEMENT; @@ -1075,7 +1075,7 @@ skip_children: xmlNodePtr tmp; if (reader->entNr == 0) { while ((tmp = node->last) != NULL) { - if ((tmp->extra & NODE_IS_PRESERVED) == 0) { + if ((XML_NODE_GET_EXTRA(tmp) & NODE_IS_PRESERVED) == 0) { xmlUnlinkNode(tmp); xmlTextReaderFreeNode(reader, tmp); } else @@ -1327,7 +1327,7 @@ get_next_node: if ((oldstate == XML_TEXTREADER_ELEMENT) && (reader->node->type == XML_ELEMENT_NODE) && (reader->node->children == NULL) && - ((reader->node->extra & NODE_IS_EMPTY) == 0) + ((XML_NODE_GET_EXTRA(reader->node) & NODE_IS_EMPTY) == 0) #ifdef LIBXML_XINCLUDE_ENABLED && (reader->in_xinclude <= 0) #endif @@ -1341,7 +1341,7 @@ get_next_node: xmlTextReaderValidatePop(reader); #endif /* LIBXML_REGEXP_ENABLED */ if ((reader->preserves > 0) && - (reader->node->extra & NODE_IS_SPRESERVED)) + (XML_NODE_GET_EXTRA(reader->node) & NODE_IS_SPRESERVED)) reader->preserves--; reader->node = reader->node->next; reader->state = XML_TEXTREADER_ELEMENT; @@ -1357,7 +1357,7 @@ get_next_node: (reader->node->prev != NULL) && (reader->node->prev->type != XML_DTD_NODE)) { xmlNodePtr tmp = reader->node->prev; - if ((tmp->extra & NODE_IS_PRESERVED) == 0) { + if ((XML_NODE_GET_EXTRA(tmp) & NODE_IS_PRESERVED) == 0) { if (oldnode == tmp) oldnode = NULL; xmlUnlinkNode(tmp); @@ -1370,7 +1370,7 @@ get_next_node: if ((oldstate == XML_TEXTREADER_ELEMENT) && (reader->node->type == XML_ELEMENT_NODE) && (reader->node->children == NULL) && - ((reader->node->extra & NODE_IS_EMPTY) == 0)) {; + ((XML_NODE_GET_EXTRA(reader->node) & NODE_IS_EMPTY) == 0)) {; reader->state = XML_TEXTREADER_END; goto node_found; } @@ -1379,7 +1379,7 @@ get_next_node: xmlTextReaderValidatePop(reader); #endif /* LIBXML_REGEXP_ENABLED */ if ((reader->preserves > 0) && - (reader->node->extra & NODE_IS_SPRESERVED)) + (XML_NODE_GET_EXTRA(reader->node) & NODE_IS_SPRESERVED)) reader->preserves--; reader->node = reader->node->parent; if ((reader->node == NULL) || @@ -1403,7 +1403,7 @@ get_next_node: #endif (reader->entNr == 0) && (oldnode->type != XML_DTD_NODE) && - ((oldnode->extra & NODE_IS_PRESERVED) == 0)) { + ((XML_NODE_GET_EXTRA(oldnode) & NODE_IS_PRESERVED) == 0)) { xmlUnlinkNode(oldnode); xmlTextReaderFreeNode(reader, oldnode); } @@ -1416,7 +1416,7 @@ get_next_node: #endif (reader->entNr == 0) && (reader->node->last != NULL) && - ((reader->node->last->extra & NODE_IS_PRESERVED) == 0)) { + ((XML_NODE_GET_EXTRA(reader->node->last) & NODE_IS_PRESERVED) == 0)) { xmlNodePtr tmp = reader->node->last; xmlUnlinkNode(tmp); xmlTextReaderFreeNode(reader, tmp); @@ -1600,7 +1600,7 @@ xmlTextReaderNext(xmlTextReaderPtr reade return(xmlTextReaderRead(reader)); if (reader->state == XML_TEXTREADER_END || reader->state == XML_TEXTREADER_BACKTRACK) return(xmlTextReaderRead(reader)); - if (cur->extra & NODE_IS_EMPTY) + if (XML_NODE_GET_EXTRA(cur) & NODE_IS_EMPTY) return(xmlTextReaderRead(reader)); do { ret = xmlTextReaderRead(reader); @@ -3023,7 +3023,7 @@ xmlTextReaderIsEmptyElement(xmlTextReade if (reader->in_xinclude > 0) return(1); #endif - return((reader->node->extra & NODE_IS_EMPTY) != 0); + return((XML_NODE_GET_EXTRA(reader->node) & NODE_IS_EMPTY) != 0); } /** @@ -3885,15 +3885,15 @@ xmlTextReaderPreserve(xmlTextReaderPtr r return(NULL); if ((cur->type != XML_DOCUMENT_NODE) && (cur->type != XML_DTD_NODE)) { - cur->extra |= NODE_IS_PRESERVED; - cur->extra |= NODE_IS_SPRESERVED; + XML_NODE_ADD_EXTRA(cur, NODE_IS_PRESERVED); + XML_NODE_ADD_EXTRA(cur, NODE_IS_SPRESERVED); } reader->preserves++; parent = cur->parent;; while (parent != NULL) { if (parent->type == XML_ELEMENT_NODE) - parent->extra |= NODE_IS_PRESERVED; + XML_NODE_ADD_EXTRA(parent, NODE_IS_PRESERVED); parent = parent->parent; } return(cur); Index: libxml2-2.11.6/xmlschemastypes.c =================================================================== --- libxml2-2.11.6.orig/xmlschemastypes.c +++ libxml2-2.11.6/xmlschemastypes.c @@ -2990,7 +2990,7 @@ xmlSchemaValAtomicType(xmlSchemaTypePtr /* * NOTE: the IDness might have already be declared in the DTD */ - if (attr->atype != XML_ATTRIBUTE_ID) { + if (XML_ATTR_GET_ATYPE(attr) != XML_ATTRIBUTE_ID) { xmlIDPtr res; xmlChar *strip; @@ -3028,7 +3028,7 @@ xmlSchemaValAtomicType(xmlSchemaTypePtr xmlFree(strip); } else xmlAddRef(NULL, node->doc, value, attr); - attr->atype = XML_ATTRIBUTE_IDREF; + XML_ATTR_SET_ATYPE(attr, XML_ATTRIBUTE_IDREF); } goto done; case XML_SCHEMAS_IDREFS: @@ -3042,7 +3042,7 @@ xmlSchemaValAtomicType(xmlSchemaTypePtr (node->type == XML_ATTRIBUTE_NODE)) { xmlAttrPtr attr = (xmlAttrPtr) node; - attr->atype = XML_ATTRIBUTE_IDREFS; + XML_ATTR_SET_ATYPE(attr, XML_ATTRIBUTE_IDREFS); } goto done; case XML_SCHEMAS_ENTITY:{ @@ -3073,7 +3073,7 @@ xmlSchemaValAtomicType(xmlSchemaTypePtr (node->type == XML_ATTRIBUTE_NODE)) { xmlAttrPtr attr = (xmlAttrPtr) node; - attr->atype = XML_ATTRIBUTE_ENTITY; + XML_ATTR_SET_ATYPE(attr, XML_ATTRIBUTE_ENTITY); } goto done; } @@ -3090,7 +3090,7 @@ xmlSchemaValAtomicType(xmlSchemaTypePtr (node->type == XML_ATTRIBUTE_NODE)) { xmlAttrPtr attr = (xmlAttrPtr) node; - attr->atype = XML_ATTRIBUTE_ENTITIES; + XML_ATTR_SET_ATYPE(attr, XML_ATTRIBUTE_ENTITIES); } goto done; case XML_SCHEMAS_NOTATION:{ Index: libxml2-2.11.6/xmlschemas.c =================================================================== --- libxml2-2.11.6.orig/xmlschemas.c +++ libxml2-2.11.6/xmlschemas.c @@ -6032,7 +6032,7 @@ xmlSchemaPValAttrNodeID(xmlSchemaParserC /* * NOTE: the IDness might have already be declared in the DTD */ - if (attr->atype != XML_ATTRIBUTE_ID) { + if (XML_ATTR_GET_ATYPE(attr) != XML_ATTRIBUTE_ID) { xmlIDPtr res; xmlChar *strip;