From d012eb80e53a31198b2eb9ab6bc109835b060611 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 27 Oct 2020 11:29:20 -0700 Subject: [PATCH] Fix xmlURIEscape memory leaks. Found by running the fuzz/uri.c fuzzer under asan (internal Android bug 171610679). Always free `ret` when exiting on failure. I've moved the definition of NULLCHK down past where ret is always initialized to make it clear that this is safe. This patch also fixes the indentation of two of the NULLCHK call sites to make it more obvious that NULLCHK isn't `if`-like. --- uri.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/uri.c b/uri.c index 7e9a9f44..8204825f 100644 --- a/uri.c +++ b/uri.c @@ -1754,11 +1754,6 @@ xmlURIEscape(const xmlChar * str) xmlURIPtr uri; int ret2; -#define NULLCHK(p) if(!p) { \ - xmlURIErrMemory("escaping URI value\n"); \ - xmlFreeURI(uri); \ - return NULL; } \ - if (str == NULL) return (NULL); @@ -1780,6 +1775,12 @@ xmlURIEscape(const xmlChar * str) ret = NULL; +#define NULLCHK(p) if(!p) { \ + xmlURIErrMemory("escaping URI value\n"); \ + xmlFreeURI(uri); \ + xmlFree(ret); \ + return NULL; } \ + if (uri->scheme) { segment = xmlURIEscapeStr(BAD_CAST uri->scheme, BAD_CAST "+-."); NULLCHK(segment) @@ -1800,7 +1801,7 @@ xmlURIEscape(const xmlChar * str) if (uri->user) { segment = xmlURIEscapeStr(BAD_CAST uri->user, BAD_CAST ";:&=+$,"); NULLCHK(segment) - ret = xmlStrcat(ret,BAD_CAST "//"); + ret = xmlStrcat(ret,BAD_CAST "//"); ret = xmlStrcat(ret, segment); ret = xmlStrcat(ret, BAD_CAST "@"); xmlFree(segment); @@ -1809,8 +1810,8 @@ xmlURIEscape(const xmlChar * str) if (uri->server) { segment = xmlURIEscapeStr(BAD_CAST uri->server, BAD_CAST "/?;:@"); NULLCHK(segment) - if (uri->user == NULL) - ret = xmlStrcat(ret, BAD_CAST "//"); + if (uri->user == NULL) + ret = xmlStrcat(ret, BAD_CAST "//"); ret = xmlStrcat(ret, segment); xmlFree(segment); } -- 2.29.0.rc2.309.g374f81d7ae-goog