rpms/logrotate
7
0
Fork 0
Code Issues Pull Requests Releases Activity

fix memory corruption issues in config file parsing

Browse Source 
Resolves: RHEL-91449
This commit is contained in:
Jan Macku 2025-06-06 15:01:47 +02:00
parent 039cb75506
commit 80dd0e796d
7 changed files with 437 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
0011-logrotate-3.18.0-Log-all-glob-errors-in-debug-mode.patch Normal file
View File

@ -0,0 +1,38 @@
From 5bbd4c6f55b8ce61953803d05e091897d26a2e7c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
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

40
0012-logrotate-3.18.0-Handle-glob-aborts-for-initial-pattern.patch Normal file
View File

@ -0,0 +1,40 @@
From ff9b66a455b890f86d38dbb772e295fa183733e4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
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

145
0013-logrotate-3.18.0-Handle-non-NUL-terminated-config-files.patch Normal file
View File

@ -0,0 +1,145 @@
From dd1562ebb0a205f8d47ff85b66a6d5b4e7a9e113 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
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

43
0014-logrotate-3.18.0-Limit-glob-length-to-avoid-stack-overflow-in-glob-3.patch Normal file
View File

@ -0,0 +1,43 @@
From f9a14b026c5c771a0bc89e204f96d7ca4d112db6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
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

110
0015-logrotate-3.18.0-Avoid-date-format-overflow-issues.patch Normal file
View File

@ -0,0 +1,110 @@
From 153ab5af4e48e4cf29a26eec8d0a3fe478c1cc43 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
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

49
0016-logrotate-3.18.0-Use-strnlen-in-private-strndup-implementation.patch Normal file
View File

@ -0,0 +1,49 @@
From 83f251b7537cb5da1fb5fa12bee68f22643420dd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
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

13
logrotate.spec
View File

@ -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 <jamacku@redhat.com> - 3.18.0-11
- fix memory corruption issues in config file parsing (RHEL-91449)
* Fri May 30 2025 Jan Macku <jamacku@redhat.com> - 3.18.0-10
- allow to disable state management (RHEL-69959)