shaba/libxml2
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/libxml2.git synced 2025-03-27 18:50:07 +03:00
Code

malloc-fail: Stop using XPath stack frames

Browse Source 
There's too much code which assumes that if ctxt->value is non-null,
a value can be successfully popped off the stack. This assumption can
break with stack frames when malloc fails.

Instead of trying to fix all call sites, remove the stack frame logic.
It only offered very little protection against misbehaving extension
functions. We already check the stack size after a function call which
should be enough.

Found by OSS-Fuzz.
This commit is contained in:
Nick Wellnhofer 2023-03-13 17:11:27 +01:00
parent 457fc622d5
commit 483793940c
2 changed files with 5 additions and 54 deletions
include/libxml
xpath.h
xpath.c

2
include/libxml/xpath.h

@ -400,7 +400,7 @@ struct _xmlXPathParserContext {
int xptr; /* it this an XPointer expression */
xmlNodePtr ancestor; /* used for walking preceding axis */
int valueFrame; /* used to limit Pop on the stack */
int valueFrame; /* unused */
};
/************************************************************************

57
xpath.c

@ -2833,42 +2833,6 @@ xmlXPathCacheConvertNumber(xmlXPathContextPtr ctxt, xmlXPathObjectPtr val) {
* *
************************************************************************/
/**
* xmlXPathSetFrame:
* @ctxt: an XPath parser context
*
* Set the callee evaluation frame
*
* Returns the previous frame value to be restored once done
*/
static int
xmlXPathSetFrame(xmlXPathParserContextPtr ctxt) {
int ret;
if (ctxt == NULL)
return(0);
ret = ctxt->valueFrame;
ctxt->valueFrame = ctxt->valueNr;
return(ret);
}
/**
* xmlXPathPopFrame:
* @ctxt: an XPath parser context
* @frame: the previous frame value
*
* Remove the callee evaluation frame
*/
static void
xmlXPathPopFrame(xmlXPathParserContextPtr ctxt, int frame) {
if (ctxt == NULL)
return;
if (ctxt->valueNr < ctxt->valueFrame) {
xmlXPatherror(ctxt, __FILE__, __LINE__, XPATH_STACK_ERROR);
}
ctxt->valueFrame = frame;
}
/**
* valuePop:
* @ctxt: an XPath evaluation context
@ -2885,11 +2849,6 @@ valuePop(xmlXPathParserContextPtr ctxt)
if ((ctxt == NULL) || (ctxt->valueNr <= 0))
return (NULL);
if (ctxt->valueNr <= ctxt->valueFrame) {
xmlXPatherror(ctxt, __FILE__, __LINE__, XPATH_STACK_ERROR);
return (NULL);
}
ctxt->valueNr--;
if (ctxt->valueNr > 0)
ctxt->value = ctxt->valueTab[ctxt->valueNr - 1];
@ -6325,7 +6284,6 @@ xmlXPathCompParserContext(xmlXPathCompExprPtr comp, xmlXPathContextPtr ctxt) {
ret->valueNr = 0;
ret->valueMax = 10;
ret->value = NULL;
ret->valueFrame = 0;
ret->context = ctxt;
ret->comp = comp;
@ -13205,20 +13163,17 @@ xmlXPathCompOpEval(xmlXPathParserContextPtr ctxt, xmlXPathStepOpPtr op)
int i;
int frame;
frame = xmlXPathSetFrame(ctxt);
frame = ctxt->valueNr;
if (op->ch1 != -1) {
total +=
xmlXPathCompOpEval(ctxt, &comp->steps[op->ch1]);
if (ctxt->error != XPATH_EXPRESSION_OK) {
xmlXPathPopFrame(ctxt, frame);
if (ctxt->error != XPATH_EXPRESSION_OK)
break;
}
}
if (ctxt->valueNr < ctxt->valueFrame + op->value) {
if (ctxt->valueNr < frame + op->value) {
xmlGenericError(xmlGenericErrorContext,
"xmlXPathCompOpEval: parameter error\n");
ctxt->error = XPATH_INVALID_OPERAND;
xmlXPathPopFrame(ctxt, frame);
break;
}
for (i = 0; i < op->value; i++) {
@ -13226,7 +13181,6 @@ xmlXPathCompOpEval(xmlXPathParserContextPtr ctxt, xmlXPathStepOpPtr op)
xmlGenericError(xmlGenericErrorContext,
"xmlXPathCompOpEval: parameter error\n");
ctxt->error = XPATH_INVALID_OPERAND;
xmlXPathPopFrame(ctxt, frame);
break;
}
}
@ -13245,7 +13199,6 @@ xmlXPathCompOpEval(xmlXPathParserContextPtr ctxt, xmlXPathStepOpPtr op)
xmlGenericError(xmlGenericErrorContext,
"xmlXPathCompOpEval: function %s bound to undefined prefix %s\n",
(char *)op->value4, (char *)op->value5);
xmlXPathPopFrame(ctxt, frame);
ctxt->error = XPATH_UNDEF_PREFIX_ERROR;
break;
}
@ -13269,9 +13222,8 @@ xmlXPathCompOpEval(xmlXPathParserContextPtr ctxt, xmlXPathStepOpPtr op)
ctxt->context->function = oldFunc;
ctxt->context->functionURI = oldFuncURI;
if ((ctxt->error == XPATH_EXPRESSION_OK) &&
(ctxt->valueNr != ctxt->valueFrame + 1))
(ctxt->valueNr != frame + 1))
XP_ERROR0(XPATH_STACK_ERROR);
xmlXPathPopFrame(ctxt, frame);
break;
}
case XPATH_OP_ARG:
@ -13952,7 +13904,6 @@ xmlXPathRunEval(xmlXPathParserContextPtr ctxt, int toBool)
ctxt->valueNr = 0;
ctxt->valueMax = 10;
ctxt->value = NULL;
ctxt->valueFrame = 0;
}
#ifdef XPATH_STREAMING
if (ctxt->comp->stream) {