Skip to content

Commit d697903

Browse files
Brian C. Youngandi34
authored andcommitted
DO NOT MERGE: Disallow namespace nodes in XPointer ranges
Namespace nodes must be copied to avoid use-after-free errors. But they don't necessarily have a physical representation in a document, so simply disallow them in XPointer ranges. Found with afl-fuzz. Fixes CVE-2016-4658. Bug: 36554207 Change-Id: Ie570c4a53ae8ca82ed4ca19701ab7d8ba9b0468f (cherry picked from commit cde4b40a9c17aec816c6b2577250fff9354a6f3c)
1 parent f8ebef2 commit d697903

File tree

1 file changed

+56
-93
lines changed

1 file changed

+56
-93
lines changed

xpointer.c

Lines changed: 56 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,45 @@ xmlXPtrRangesEqual(xmlXPathObjectPtr range1, xmlXPathObjectPtr range2) {
319319
return(1);
320320
}
321321

322+
/**
323+
* xmlXPtrNewRangeInternal:
324+
* @start: the starting node
325+
* @startindex: the start index
326+
* @end: the ending point
327+
* @endindex: the ending index
328+
*
329+
* Internal function to create a new xmlXPathObjectPtr of type range
330+
*
331+
* Returns the newly created object.
332+
*/
333+
static xmlXPathObjectPtr
334+
xmlXPtrNewRangeInternal(xmlNodePtr start, int startindex,
335+
xmlNodePtr end, int endindex) {
336+
xmlXPathObjectPtr ret;
337+
338+
/*
339+
* Namespace nodes must be copied (see xmlXPathNodeSetDupNs).
340+
* Disallow them for now.
341+
*/
342+
if ((start != NULL) && (start->type == XML_NAMESPACE_DECL))
343+
return(NULL);
344+
if ((end != NULL) && (end->type == XML_NAMESPACE_DECL))
345+
return(NULL);
346+
347+
ret = (xmlXPathObjectPtr) xmlMalloc(sizeof(xmlXPathObject));
348+
if (ret == NULL) {
349+
xmlXPtrErrMemory("allocating range");
350+
return(NULL);
351+
}
352+
memset(ret, 0, sizeof(xmlXPathObject));
353+
ret->type = XPATH_RANGE;
354+
ret->user = start;
355+
ret->index = startindex;
356+
ret->user2 = end;
357+
ret->index2 = endindex;
358+
return(ret);
359+
}
360+
322361
/**
323362
* xmlXPtrNewRange:
324363
* @start: the starting node
@@ -344,17 +383,7 @@ xmlXPtrNewRange(xmlNodePtr start, int startindex,
344383
if (endindex < 0)
345384
return(NULL);
346385

347-
ret = (xmlXPathObjectPtr) xmlMalloc(sizeof(xmlXPathObject));
348-
if (ret == NULL) {
349-
xmlXPtrErrMemory("allocating range");
350-
return(NULL);
351-
}
352-
memset(ret, 0 , (size_t) sizeof(xmlXPathObject));
353-
ret->type = XPATH_RANGE;
354-
ret->user = start;
355-
ret->index = startindex;
356-
ret->user2 = end;
357-
ret->index2 = endindex;
386+
ret = xmlXPtrNewRangeInternal(start, startindex, end, endindex);
358387
xmlXPtrRangeCheckOrder(ret);
359388
return(ret);
360389
}
@@ -381,17 +410,8 @@ xmlXPtrNewRangePoints(xmlXPathObjectPtr start, xmlXPathObjectPtr end) {
381410
if (end->type != XPATH_POINT)
382411
return(NULL);
383412

384-
ret = (xmlXPathObjectPtr) xmlMalloc(sizeof(xmlXPathObject));
385-
if (ret == NULL) {
386-
xmlXPtrErrMemory("allocating range");
387-
return(NULL);
388-
}
389-
memset(ret, 0 , (size_t) sizeof(xmlXPathObject));
390-
ret->type = XPATH_RANGE;
391-
ret->user = start->user;
392-
ret->index = start->index;
393-
ret->user2 = end->user;
394-
ret->index2 = end->index;
413+
ret = xmlXPtrNewRangeInternal(start->user, start->index, end->user,
414+
end->index);
395415
xmlXPtrRangeCheckOrder(ret);
396416
return(ret);
397417
}
@@ -416,17 +436,7 @@ xmlXPtrNewRangePointNode(xmlXPathObjectPtr start, xmlNodePtr end) {
416436
if (start->type != XPATH_POINT)
417437
return(NULL);
418438

419-
ret = (xmlXPathObjectPtr) xmlMalloc(sizeof(xmlXPathObject));
420-
if (ret == NULL) {
421-
xmlXPtrErrMemory("allocating range");
422-
return(NULL);
423-
}
424-
memset(ret, 0 , (size_t) sizeof(xmlXPathObject));
425-
ret->type = XPATH_RANGE;
426-
ret->user = start->user;
427-
ret->index = start->index;
428-
ret->user2 = end;
429-
ret->index2 = -1;
439+
ret = xmlXPtrNewRangeInternal(start->user, start->index, end, -1);
430440
xmlXPtrRangeCheckOrder(ret);
431441
return(ret);
432442
}
@@ -453,17 +463,7 @@ xmlXPtrNewRangeNodePoint(xmlNodePtr start, xmlXPathObjectPtr end) {
453463
if (end->type != XPATH_POINT)
454464
return(NULL);
455465

456-
ret = (xmlXPathObjectPtr) xmlMalloc(sizeof(xmlXPathObject));
457-
if (ret == NULL) {
458-
xmlXPtrErrMemory("allocating range");
459-
return(NULL);
460-
}
461-
memset(ret, 0 , (size_t) sizeof(xmlXPathObject));
462-
ret->type = XPATH_RANGE;
463-
ret->user = start;
464-
ret->index = -1;
465-
ret->user2 = end->user;
466-
ret->index2 = end->index;
466+
ret = xmlXPtrNewRangeInternal(start, -1, end->user, end->index);
467467
xmlXPtrRangeCheckOrder(ret);
468468
return(ret);
469469
}
@@ -486,17 +486,7 @@ xmlXPtrNewRangeNodes(xmlNodePtr start, xmlNodePtr end) {
486486
if (end == NULL)
487487
return(NULL);
488488

489-
ret = (xmlXPathObjectPtr) xmlMalloc(sizeof(xmlXPathObject));
490-
if (ret == NULL) {
491-
xmlXPtrErrMemory("allocating range");
492-
return(NULL);
493-
}
494-
memset(ret, 0 , (size_t) sizeof(xmlXPathObject));
495-
ret->type = XPATH_RANGE;
496-
ret->user = start;
497-
ret->index = -1;
498-
ret->user2 = end;
499-
ret->index2 = -1;
489+
ret = xmlXPtrNewRangeInternal(start, -1, end, -1);
500490
xmlXPtrRangeCheckOrder(ret);
501491
return(ret);
502492
}
@@ -516,17 +506,7 @@ xmlXPtrNewCollapsedRange(xmlNodePtr start) {
516506
if (start == NULL)
517507
return(NULL);
518508

519-
ret = (xmlXPathObjectPtr) xmlMalloc(sizeof(xmlXPathObject));
520-
if (ret == NULL) {
521-
xmlXPtrErrMemory("allocating range");
522-
return(NULL);
523-
}
524-
memset(ret, 0 , (size_t) sizeof(xmlXPathObject));
525-
ret->type = XPATH_RANGE;
526-
ret->user = start;
527-
ret->index = -1;
528-
ret->user2 = NULL;
529-
ret->index2 = -1;
509+
ret = xmlXPtrNewRangeInternal(start, -1, NULL, -1);
530510
return(ret);
531511
}
532512

@@ -541,6 +521,8 @@ xmlXPtrNewCollapsedRange(xmlNodePtr start) {
541521
*/
542522
xmlXPathObjectPtr
543523
xmlXPtrNewRangeNodeObject(xmlNodePtr start, xmlXPathObjectPtr end) {
524+
xmlNodePtr endNode;
525+
int endIndex;
544526
xmlXPathObjectPtr ret;
545527

546528
if (start == NULL)
@@ -549,47 +531,28 @@ xmlXPtrNewRangeNodeObject(xmlNodePtr start, xmlXPathObjectPtr end) {
549531
return(NULL);
550532
switch (end->type) {
551533
case XPATH_POINT:
534+
endNode = end->user;
535+
endIndex = end->index;
536+
break;
552537
case XPATH_RANGE:
538+
endNode = end->user2;
539+
endIndex = end->index2;
553540
break;
554541
case XPATH_NODESET:
555542
/*
556543
* Empty set ...
557544
*/
558545
if (end->nodesetval->nodeNr <= 0)
559546
return(NULL);
547+
endNode = end->nodesetval->nodeTab[end->nodesetval->nodeNr - 1];
548+
endIndex = -1;
560549
break;
561550
default:
562551
/* TODO */
563552
return(NULL);
564553
}
565554

566-
ret = (xmlXPathObjectPtr) xmlMalloc(sizeof(xmlXPathObject));
567-
if (ret == NULL) {
568-
xmlXPtrErrMemory("allocating range");
569-
return(NULL);
570-
}
571-
memset(ret, 0 , (size_t) sizeof(xmlXPathObject));
572-
ret->type = XPATH_RANGE;
573-
ret->user = start;
574-
ret->index = -1;
575-
switch (end->type) {
576-
case XPATH_POINT:
577-
ret->user2 = end->user;
578-
ret->index2 = end->index;
579-
break;
580-
case XPATH_RANGE:
581-
ret->user2 = end->user2;
582-
ret->index2 = end->index2;
583-
break;
584-
case XPATH_NODESET: {
585-
ret->user2 = end->nodesetval->nodeTab[end->nodesetval->nodeNr - 1];
586-
ret->index2 = -1;
587-
break;
588-
}
589-
default:
590-
STRANGE
591-
return(NULL);
592-
}
555+
ret = xmlXPtrNewRangeInternal(start, -1, endNode, endIndex);
593556
xmlXPtrRangeCheckOrder(ret);
594557
return(ret);
595558
}

0 commit comments

Comments
 (0)