1
0
mirror of https://gitlab.gnome.org/GNOME/libxml2.git synced 2025-09-09 17:44:14 +03:00

malloc-fail: Avoid use-after-free after unsuccessful valuePush

In xpath.c there's a lot of code like:

    valuePush(ctxt, xmlCacheNewX());
    ...
    valuePop(ctxt);

If xmlCacheNewX fails, no value will be pushed on the stack. If there's
no error check in between, valuePop will pop an unrelated value which
can lead to use-after-free errors.

Instead of trying to fix all call sites, we simply stop popping values
if an error was signaled. This requires to change the CHECK_TYPE macro
which is often used to determine whether a value can be safely popped.

Found with libFuzzer, see #344.
This commit is contained in:
Nick Wellnhofer
2023-01-31 12:46:30 +01:00
parent 7ec314efcd
commit 6a12be77c6
2 changed files with 11 additions and 2 deletions

View File

@@ -273,7 +273,8 @@ XMLPUBFUN void *
* type.
*/
#define CHECK_TYPE(typeval) \
if ((ctxt->value == NULL) || (ctxt->value->type != typeval)) \
if ((ctxt->error != 0) || \
(ctxt->value == NULL) || (ctxt->value->type != typeval)) \
XP_ERROR(XPATH_INVALID_TYPE)
/**

10
xpath.c
View File

@@ -2873,7 +2873,15 @@ valuePop(xmlXPathParserContextPtr ctxt)
{
xmlXPathObjectPtr ret;
if ((ctxt == NULL) || (ctxt->valueNr <= 0))
/*
* If a memory allocation failed, it can happen that valuePush doesn't
* push a value on the stack. If there's no error check before the
* corresponding valuePop call, we would pop an unrelated object which
* could lead to use-after-free errors later on. So we don't pop values
* if an error was signaled. The stack will be cleaned later in
* xmlXPathFreeParserContext.
*/
if ((ctxt == NULL) || (ctxt->valueNr <= 0) || (ctxt->error != 0))
return (NULL);
if (ctxt->valueNr <= ctxt->valueFrame) {