From 80dd0e796d1b3f27f038bbf7bb4be5e74b47722e Mon Sep 17 00:00:00 2001 From: Jan Macku Date: Fri, 6 Jun 2025 15:01:47 +0200 Subject: [PATCH] fix memory corruption issues in config file parsing Resolves: RHEL-91449 --- ....0-Log-all-glob-errors-in-debug-mode.patch | 38 +++++ ...ndle-glob-aborts-for-initial-pattern.patch | 40 +++++ ...ndle-non-NUL-terminated-config-files.patch | 145 ++++++++++++++++++ ...th-to-avoid-stack-overflow-in-glob-3.patch | 43 ++++++ ....0-Avoid-date-format-overflow-issues.patch | 110 +++++++++++++ ...en-in-private-strndup-implementation.patch | 49 ++++++ logrotate.spec | 13 +- 7 files changed, 437 insertions(+), 1 deletion(-) create mode 100644 0011-logrotate-3.18.0-Log-all-glob-errors-in-debug-mode.patch create mode 100644 0012-logrotate-3.18.0-Handle-glob-aborts-for-initial-pattern.patch create mode 100644 0013-logrotate-3.18.0-Handle-non-NUL-terminated-config-files.patch create mode 100644 0014-logrotate-3.18.0-Limit-glob-length-to-avoid-stack-overflow-in-glob-3.patch create mode 100644 0015-logrotate-3.18.0-Avoid-date-format-overflow-issues.patch create mode 100644 0016-logrotate-3.18.0-Use-strnlen-in-private-strndup-implementation.patch diff --git a/0011-logrotate-3.18.0-Log-all-glob-errors-in-debug-mode.patch b/0011-logrotate-3.18.0-Log-all-glob-errors-in-debug-mode.patch new file mode 100644 index 0000000..4e0a559 --- /dev/null +++ b/0011-logrotate-3.18.0-Log-all-glob-errors-in-debug-mode.patch @@ -0,0 +1,38 @@ +From 5bbd4c6f55b8ce61953803d05e091897d26a2e7c Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= +Date: Wed, 12 Jul 2023 21:47:45 +0200 +Subject: [PATCH 1/6] Log all glob errors in debug mode + +In case glob(3) returns GLOB_ABORTED always log the error message in +debug mode, since it might be overridden by a further pattern from the +same file statement. + +Also log an OOM message if asprintf(3) fails. + +Reported-by: blu3sh0rk +(cherry picked from commit 7d6ecd67e2c2ab99e26693a0347a2361d11c51e1) +--- + config.c | 6 +++++- + 1 file changed, 5 insertions(+), 1 deletion(-) + +diff --git a/config.c b/config.c +index e76fad0..96f34f8 100644 +--- a/config.c ++++ b/config.c +@@ -1794,8 +1794,12 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig) + * set, so store the error message for later. */ + rc = asprintf(&globerr_msg, "%s:%d glob failed for %s: %s\n", + configFile, lineNum, argv[argNum], strerror(glob_errno)); +- if (rc == -1) ++ if (rc == -1) { ++ message_OOM(); + globerr_msg = NULL; ++ } else { ++ message(MESS_DEBUG, "%s", globerr_msg); ++ } + + globResult.gl_pathc = 0; + } +-- +2.49.0 + diff --git a/0012-logrotate-3.18.0-Handle-glob-aborts-for-initial-pattern.patch b/0012-logrotate-3.18.0-Handle-glob-aborts-for-initial-pattern.patch new file mode 100644 index 0000000..914520d --- /dev/null +++ b/0012-logrotate-3.18.0-Handle-glob-aborts-for-initial-pattern.patch @@ -0,0 +1,40 @@ +From ff9b66a455b890f86d38dbb772e295fa183733e4 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= +Date: Wed, 12 Jul 2023 21:47:52 +0200 +Subject: [PATCH 2/6] Handle glob aborts for initial pattern + +In case glob(3) fails with GLOB_ABORTED, e.g. due to missing file +permissions, the number of path matches gets set to 0. If the number of +path matches is 0 and there have been no other files matched yet the +following realloc(3) call will be called with a size of 0, free'ing the +array. Since the array gets only assigned to the realloc(3) result in +the non NULL case, the free'd array pointer is retained and any further +access, e.g. by a future glob result, will result in a use-after-free. + +Reported-by: blu3sh0rk +(cherry picked from commit f444a9858e306c94db37f9d7ddbae817530e949e) +--- + config.c | 7 +++++++ + 1 file changed, 7 insertions(+) + +diff --git a/config.c b/config.c +index 96f34f8..33e283c 100644 +--- a/config.c ++++ b/config.c +@@ -1804,6 +1804,13 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig) + globResult.gl_pathc = 0; + } + ++ if (globResult.gl_pathc == 0) { ++ message(MESS_DEBUG, "%s:%d no matches for glob '%s', skipping\n", ++ configFile, lineNum, argv[argNum]); ++ globfree(&globResult); ++ continue; ++ } ++ + tmp = realloc(newlog->files, + sizeof(*newlog->files) * (newlog->numFiles + + globResult. +-- +2.49.0 + diff --git a/0013-logrotate-3.18.0-Handle-non-NUL-terminated-config-files.patch b/0013-logrotate-3.18.0-Handle-non-NUL-terminated-config-files.patch new file mode 100644 index 0000000..195e517 --- /dev/null +++ b/0013-logrotate-3.18.0-Handle-non-NUL-terminated-config-files.patch @@ -0,0 +1,145 @@ +From dd1562ebb0a205f8d47ff85b66a6d5b4e7a9e113 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= +Date: Wed, 12 Jul 2023 21:47:54 +0200 +Subject: [PATCH 3/6] Handle non NUL terminated config files + +Avoid calling strndup(3) on pointers of the mapped config file in case +the memory is not NUL terminated. + +Validate the current buffer location before dereferencing it if it might +have been advanced after the check in the loop condition. + +Reported-by: blu3sh0rk +(cherry picked from commit 39735f94bed266af08c047aa021d34682ff47433) +--- + config.c | 35 +++++++++++++++++++++++++---------- + 1 file changed, 25 insertions(+), 10 deletions(-) + +diff --git a/config.c b/config.c +index 33e283c..38ef0b0 100644 +--- a/config.c ++++ b/config.c +@@ -159,6 +159,7 @@ static char *isolateLine(char **strt, char **buf, size_t length) { + char *endtag, *start, *tmp; + const char *max = *buf + length; + char *key; ++ size_t llen; + + start = *strt; + endtag = start; +@@ -167,13 +168,19 @@ static char *isolateLine(char **strt, char **buf, size_t length) { + if (max < endtag) + return NULL; + tmp = endtag - 1; +- while (isspace((unsigned char)*endtag)) ++ while (endtag >= start && endtag < max && isspace((unsigned char)*endtag)) + endtag--; +- key = strndup(start, (size_t)(endtag - start + 1)); ++ llen = (size_t)(endtag - start + 1); ++ if (start + llen > max) ++ llen = (size_t)(max - start); ++ /* Avoid strndup(3) since the buffer might not be NUL-terminated. */ ++ key = malloc(llen + 1); + if (key == NULL) { + message_OOM(); + return NULL; + } ++ memcpy(key, start, llen); ++ key[llen] = '\0'; + *strt = tmp; + return key; + } +@@ -182,7 +189,7 @@ static char *isolateValue(const char *fileName, int lineNum, const char *key, + char **startPtr, char **buf, size_t length) + { + char *chptr = *startPtr; +- const char *max = *startPtr + length; ++ const char *max = *buf + length; + + while (chptr < max && isblank((unsigned char)*chptr)) + chptr++; +@@ -206,6 +213,8 @@ static char *isolateWord(char **strt, char **buf, size_t length) { + char *endtag, *start; + const char *max = *buf + length; + char *key; ++ size_t wlen; ++ + start = *strt; + while (start < max && isblank((unsigned char)*start)) + start++; +@@ -214,11 +223,15 @@ static char *isolateWord(char **strt, char **buf, size_t length) { + endtag++;} + if (max < endtag) + return NULL; +- key = strndup(start, (size_t)(endtag - start)); ++ wlen = (size_t)(endtag - start); ++ /* Avoid strndup(3) since the buffer might not be NUL-terminated. */ ++ key = malloc(wlen + 1); + if (key == NULL) { + message_OOM(); + return NULL; + } ++ memcpy(key, start, wlen); ++ key[wlen] = '\0'; + *strt = endtag; + return key; + } +@@ -1091,7 +1104,9 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig) + configFile, lineNum); + RAISE_ERROR(); + } +- if (!isspace((unsigned char)*start) && *start != '=') { ++ if (start < buf + length && ++ !isspace((unsigned char)*start) && ++ *start != '=') { + message(MESS_ERROR, "%s:%d keyword '%s' not properly" + " separated, found %#x\n", + configFile, lineNum, key, *start); +@@ -1709,7 +1724,7 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig) + } else { + message(MESS_ERROR, "%s:%d unknown option '%s' " + "-- ignoring line\n", configFile, lineNum, key); +- if (*start != '\n') ++ if (start < buf + length && *start != '\n') + state = STATE_SKIP_LINE; + } + } else if (*start == '/' || *start == '"' || *start == '\'' +@@ -2048,7 +2063,7 @@ duperror: + } + else { + const char *endtag = start - 9; +- while (*endtag != '\n') ++ while (endtag > scriptStart && *endtag != '\n') + endtag--; + endtag++; + *scriptDest = strndup(scriptStart, (size_t)(endtag - scriptStart)); +@@ -2063,7 +2078,7 @@ duperror: + state = (state & STATE_SKIP_CONFIG) ? STATE_SKIP_CONFIG : STATE_DEFAULT; + } + else { +- state = (*start == '\n' ? 0 : STATE_SKIP_LINE) | ++ state = (start < buf + length && *start == '\n' ? 0 : STATE_SKIP_LINE) | + STATE_LOAD_SCRIPT | + ((state & STATE_SKIP_CONFIG) ? STATE_SKIP_CONFIG : 0); + } +@@ -2103,7 +2118,7 @@ duperror: + * pointer is increased by one and, after this, + * "start" points to the beginning of the next line. + */ +- if (*start != '\n') { ++ if (start < buf + length && *start != '\n') { + state = STATE_SKIP_LINE | STATE_SKIP_CONFIG; + } + } +@@ -2114,7 +2129,7 @@ duperror: + "%s: %d: readConfigFile() unknown state\n", + configFile, lineNum); + } +- if (*start == '\n') { ++ if (start < buf + length && *start == '\n') { + lineNum++; + } + +-- +2.49.0 + diff --git a/0014-logrotate-3.18.0-Limit-glob-length-to-avoid-stack-overflow-in-glob-3.patch b/0014-logrotate-3.18.0-Limit-glob-length-to-avoid-stack-overflow-in-glob-3.patch new file mode 100644 index 0000000..428a082 --- /dev/null +++ b/0014-logrotate-3.18.0-Limit-glob-length-to-avoid-stack-overflow-in-glob-3.patch @@ -0,0 +1,43 @@ +From f9a14b026c5c771a0bc89e204f96d7ca4d112db6 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= +Date: Wed, 12 Jul 2023 21:47:56 +0200 +Subject: [PATCH 4/6] Limit glob length to avoid stack overflow in glob(3) + +Limit the supported length of glob pattern to 2048 to avoid stack +overflows inside glob(3) due to recursion. + +Reported-by: blu3sh0rk +(cherry picked from commit 0271501ae37b1455b98abc00b9bb77096610462b) +--- + config.c | 8 ++++++++ + 1 file changed, 8 insertions(+) + +diff --git a/config.c b/config.c +index 38ef0b0..b213b38 100644 +--- a/config.c ++++ b/config.c +@@ -1787,6 +1787,7 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig) + newlog->numFiles = 0; + for (argNum = 0; argNum < argc; argNum++) { + char **tmp; ++ size_t argLen = strlen(argv[argNum]); + int rc; + glob_t globResult; + +@@ -1795,6 +1796,13 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig) + globerr_msg = NULL; + } + ++ if (argLen > 2048) { ++ message(MESS_ERROR, "%s:%d glob too long (%zu > 2048)\n", ++ configFile, lineNum, argLen); ++ logerror = 1; ++ continue; ++ } ++ + rc = glob(argv[argNum], GLOB_NOCHECK + #ifdef GLOB_TILDE + | GLOB_TILDE +-- +2.49.0 + diff --git a/0015-logrotate-3.18.0-Avoid-date-format-overflow-issues.patch b/0015-logrotate-3.18.0-Avoid-date-format-overflow-issues.patch new file mode 100644 index 0000000..3eee414 --- /dev/null +++ b/0015-logrotate-3.18.0-Avoid-date-format-overflow-issues.patch @@ -0,0 +1,110 @@ +From 153ab5af4e48e4cf29a26eec8d0a3fe478c1cc43 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= +Date: Wed, 12 Jul 2023 21:47:59 +0200 +Subject: [PATCH 5/6] Avoid date format overflow issues + +Prevent overflows on the date format string array. + +Exit early on date format string conversion failures to avoid operating +on wrong files or missing some out. + +Reported-by: blu3sh0rk +(cherry picked from commit 1b45002788b2838fdff32da34a2370f519f7eb51) +--- + logrotate.c | 33 ++++++++++++++++++++------------- + 1 file changed, 20 insertions(+), 13 deletions(-) + +diff --git a/logrotate.c b/logrotate.c +index 1bb95b2..b0f8332 100644 +--- a/logrotate.c ++++ b/logrotate.c +@@ -1542,6 +1542,8 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum, + char dext_str[DATEEXT_LEN]; + char dformat[PATTERN_LEN] = ""; + char dext_pattern[PATTERN_LEN]; ++ const char *final_dformat; ++ size_t ret; + + if (!state->doRotate) + return 0; +@@ -1661,19 +1663,19 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum, + /* Construct the glob pattern corresponding to the date format */ + dext_str[0] = '\0'; + if (log->dateformat) { +- char *dext; ++ const char *dext = log->dateformat; + size_t i = 0, j = 0; ++ + memset(dext_pattern, 0, sizeof(dext_pattern)); +- dext = log->dateformat; + while (*dext == ' ') + dext++; +- while ((*dext != '\0') && (!hasErrors)) { ++ while (*dext != '\0') { + /* Will there be a space for a char and '\0'? */ +- if (j >= (sizeof(dext_pattern) - 1)) { ++ if (j >= (sizeof(dext_pattern) - 1) || ++ i >= (sizeof(dformat) - 1)) { + message(MESS_ERROR, "Date format %s is too long\n", + log->dateformat); +- hasErrors = 1; +- break; ++ return 1; + } + if (*dext == '%') { + switch (*(dext + 1)) { +@@ -1694,8 +1696,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum, + if (j >= (sizeof(dext_pattern) - 1)) { + message(MESS_ERROR, "Date format %s is too long\n", + log->dateformat); +- hasErrors = 1; +- break; ++ return 1; + } + dformat[i++] = *(dext++); + dformat[i] = *dext; +@@ -1709,8 +1710,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum, + if (j >= (sizeof(dext_pattern) - 1)) { + message(MESS_ERROR, "Date format %s is too long\n", + log->dateformat); +- hasErrors = 1; +- break; ++ return 1; + } + dformat[i++] = *(dext++); + dformat[i] = *dext; +@@ -1730,21 +1730,28 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum, + } + dformat[i] = '\0'; + message(MESS_DEBUG, "Converted '%s' -> '%s'\n", log->dateformat, dformat); +- strftime(dext_str, sizeof(dext_str), dformat, &now); ++ final_dformat = dformat; + } else { + if (log->criterium == ROT_HOURLY) { + /* hourly adds another two digits */ +- strftime(dext_str, sizeof(dext_str), "-%Y%m%d%H", &now); ++ final_dformat = "-%Y%m%d%H"; + strncpy(dext_pattern, "-[0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]", + sizeof(dext_pattern)); + } else { + /* The default dateformat and glob pattern */ +- strftime(dext_str, sizeof(dext_str), "-%Y%m%d", &now); ++ final_dformat = "-%Y%m%d"; + strncpy(dext_pattern, "-[0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]", + sizeof(dext_pattern)); + } + dext_pattern[PATTERN_LEN - 1] = '\0'; + } ++ ++ ret = strftime(dext_str, sizeof(dext_str), final_dformat, &now); ++ if (ret == 0) { ++ message(MESS_ERROR, "failed to apply date format '%s'\n", final_dformat); ++ return 1; ++ } ++ + message(MESS_DEBUG, "dateext suffix '%s'\n", dext_str); + message(MESS_DEBUG, "glob pattern '%s'\n", dext_pattern); + +-- +2.49.0 + diff --git a/0016-logrotate-3.18.0-Use-strnlen-in-private-strndup-implementation.patch b/0016-logrotate-3.18.0-Use-strnlen-in-private-strndup-implementation.patch new file mode 100644 index 0000000..782ba28 --- /dev/null +++ b/0016-logrotate-3.18.0-Use-strnlen-in-private-strndup-implementation.patch @@ -0,0 +1,49 @@ +From 83f251b7537cb5da1fb5fa12bee68f22643420dd Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= +Date: Thu, 27 Jul 2023 17:42:36 +0200 +Subject: [PATCH 6/6] Use strnlen() in private strndup() implementation + +strndup() might be called on non NUL terminated buffers, thus use +strnlen(). + +Since for this code to be active strndup() has not to be supported by +the standard C library of the system, use a private version of strnlen() +as well. + +(cherry picked from commit 21c614410abe68c894804d0efbae730cb87c14bb) +--- + config.c | 13 +++++++++---- + 1 file changed, 9 insertions(+), 4 deletions(-) + +diff --git a/config.c b/config.c +index b213b38..2eddcec 100644 +--- a/config.c ++++ b/config.c +@@ -80,15 +80,20 @@ int asprintf(char **string_ptr, const char *format, ...) + #endif + + #if !defined(HAVE_STRNDUP) ++static size_t logr__strnlen(const char *s, size_t n) ++{ ++ const char *p; ++ ++ p = memchr(s, '\0', n); ++ return p ? (size_t)(p - s) : n; ++} ++ + char *strndup(const char *s, size_t n) + { + size_t nAvail; + char *p; + +- /* min() */ +- nAvail = strlen(s) + 1; +- if ( (n + 1) < nAvail) +- nAvail = n + 1; ++ nAvail = logr__strnlen(s, n) + 1; + + p = malloc(nAvail); + if (!p) +-- +2.49.0 + diff --git a/logrotate.spec b/logrotate.spec index 8ae5956..992db82 100644 --- a/logrotate.spec +++ b/logrotate.spec @@ -1,7 +1,7 @@ Summary: Rotates, compresses, removes and mails system log files Name: logrotate Version: 3.18.0 -Release: 10%{?dist} +Release: 11%{?dist} License: GPLv2+ URL: https://github.com/logrotate/logrotate Source0: https://github.com/logrotate/logrotate/releases/download/%{version}/logrotate-%{version}.tar.xz @@ -31,6 +31,14 @@ Patch: 0008-logrotate-3.18.0-test-0107-cover-the-ignoreduplicates-configuratio Patch: 0009-logrotate-3.18.0-add-documentation-for-state-dev-null-special-case.patch Patch: 0010-logrotate-3.18.0-Do-not-lock-state-file-dev-null.patch +# fix memory corruption issues in config file parsing +Patch: 0011-logrotate-3.18.0-Log-all-glob-errors-in-debug-mode.patch +Patch: 0012-logrotate-3.18.0-Handle-glob-aborts-for-initial-pattern.patch +Patch: 0013-logrotate-3.18.0-Handle-non-NUL-terminated-config-files.patch +Patch: 0014-logrotate-3.18.0-Limit-glob-length-to-avoid-stack-overflow-in-glob-3.patch +Patch: 0015-logrotate-3.18.0-Avoid-date-format-overflow-issues.patch +Patch: 0016-logrotate-3.18.0-Use-strnlen-in-private-strndup-implementation.patch + BuildRequires: acl BuildRequires: automake BuildRequires: gcc @@ -131,6 +139,9 @@ fi %config(noreplace) %{_sysconfdir}/rwtab.d/logrotate %changelog +* Fri Jun 06 2025 Jan Macku - 3.18.0-11 +- fix memory corruption issues in config file parsing (RHEL-91449) + * Fri May 30 2025 Jan Macku - 3.18.0-10 - allow to disable state management (RHEL-69959)