From f575507efbe8f876e739e8759da8ccf88d0988c5 Mon Sep 17 00:00:00 2001 From: Trent Piepho Date: Thu, 10 Jun 2021 14:11:05 -0700 Subject: [PATCH 1/2] Fix hangs when reading bad xml The xml library functions return -1 on error and 0 on EOF. A number of loops of the form "while (result)" would loop forever on an error. Change this to "while (result > 0)", so that they exit on an error. The return value of the functions in xml.c that did this will be 1 on success and 0 on EOF *or error*. So the code using the xml.c functions will be ok as is. Example that hangs: 123 The 2nd is missing the slash and doesn't terminate the element, so is unmatched and it will hang on that error. --- xml.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/xml.c b/xml.c index ba1ff21..f7ef9ae 100644 --- a/xml.c +++ b/xml.c @@ -23,11 +23,11 @@ int stepInto(xmlTextReaderPtr reader) { int result; do { result = xmlTextReaderRead(reader); - } while (result + } while (result > 0 && (xmlTextReaderNodeType(reader) == XML_READER_TYPE_SIGNIFICANT_WHITESPACE || xmlTextReaderNodeType(reader) == XML_READER_TYPE_COMMENT || xmlTextReaderNodeType(reader) == XML_READER_TYPE_END_ELEMENT)); - return result; + return result > 0; } int stepOver(xmlTextReaderPtr reader) { @@ -35,14 +35,14 @@ int stepOver(xmlTextReaderPtr reader) { int result; do { result = xmlTextReaderRead(reader); - } while (result && xmlTextReaderDepth(reader) > depth); - while (result + } while (result > 0 && xmlTextReaderDepth(reader) > depth); + while (result > 0 && (xmlTextReaderNodeType(reader) == XML_READER_TYPE_SIGNIFICANT_WHITESPACE || xmlTextReaderNodeType(reader) == XML_READER_TYPE_COMMENT || xmlTextReaderNodeType(reader) == XML_READER_TYPE_END_ELEMENT)) { result = xmlTextReaderRead(reader); } - return result; + return result > 0; } int stepOut(xmlTextReaderPtr reader) { @@ -50,27 +50,27 @@ int stepOut(xmlTextReaderPtr reader) { int result; do { result = xmlTextReaderRead(reader); - } while (result && xmlTextReaderDepth(reader) > depth); - while (result + } while (result > 0 && xmlTextReaderDepth(reader) > depth); + while (result > 0 && (xmlTextReaderNodeType(reader) == XML_READER_TYPE_SIGNIFICANT_WHITESPACE || xmlTextReaderNodeType(reader) == XML_READER_TYPE_COMMENT || xmlTextReaderNodeType(reader) == XML_READER_TYPE_END_ELEMENT)) { result = xmlTextReaderRead(reader); } - return result; + return result > 0; } int stepOverText(xmlTextReaderPtr reader, const char ** text) { int depth = xmlTextReaderDepth(reader); int result = stepInto(reader); *text = NULL; - if (result && xmlTextReaderDepth(reader) > depth) { + if (result > 0 && xmlTextReaderDepth(reader) > depth) { if (xmlTextReaderNodeType(reader) == XML_READER_TYPE_TEXT) { *text = xmlTextReaderValue(reader); } result = stepOut(reader); } - return result; + return result > 0; } int elementMatches(xmlTextReaderPtr reader, const char * namespace, const char * nodeName) { From c967cd1218c94642d2fa86ea6f5d76f06a947b8c Mon Sep 17 00:00:00 2001 From: Trent Piepho Date: Thu, 10 Jun 2021 14:08:11 -0700 Subject: [PATCH 2/2] Fix hang on unknown element in listen section The code that parsed a listen section would go into an infinite loop if it encountered a element with an unknown tag. Example: hostname It will loop forever on the typo element and not skip it. Change this to print an error and skip the element. --- configuration.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/configuration.c b/configuration.c index abf59d4..d3bc4ad 100644 --- a/configuration.c +++ b/configuration.c @@ -86,11 +86,12 @@ static int configListen(WebdavdConfiguration * config, xmlTextReaderPtr reader, if (xmlTextReaderNodeType(reader) == XML_READER_TYPE_ELEMENT && !strcmp(xmlTextReaderConstNamespaceUri(reader), CONFIG_NAMESPACE)) { - if (!strcmp(xmlTextReaderConstLocalName(reader), "port")) { + const char * elementName = xmlTextReaderConstLocalName(reader); + if (!strcmp(elementName, "port")) { result = readConfigInt(reader, &config->daemons[index].port, configFile); - } else if (!strcmp(xmlTextReaderConstLocalName(reader), "host")) { + } else if (!strcmp(elementName, "host")) { result = readConfigString(reader, &config->daemons[index].host); - } else if (!strcmp(xmlTextReaderConstLocalName(reader), "encryption")) { + } else if (!strcmp(elementName, "encryption")) { const char * encryptionString; result = stepOverText(reader, &encryptionString); if (encryptionString) { @@ -104,7 +105,7 @@ static int configListen(WebdavdConfiguration * config, xmlTextReaderPtr reader, } xmlFree((char *) encryptionString); } - } else if (!strcmp(xmlTextReaderConstLocalName(reader), "forward-to")) { + } else if (!strcmp(elementName, "forward-to")) { int depth2 = xmlTextReaderDepth(reader) + 1; result = stepInto(reader); config->daemons[index].forwardToIsEncrypted = -1; @@ -143,6 +144,9 @@ static int configListen(WebdavdConfiguration * config, xmlTextReaderPtr reader, config->daemons[index].forwardToPort == 443 ? 1 : 0; } } + } else { + stdLogError(0, "Unknown element tag '%s' in listen section", elementName); + result = stepOver(reader); } } else { result = stepOver(reader);