Compare commits

...

No commits in common. "c8" and "c9" have entirely different histories.
c8 ... c9

14 changed files with 1683 additions and 1508 deletions

2
.gitignore vendored
View File

@ -1 +1 @@
SOURCES/logrotate-3.14.0.tar.xz
SOURCES/logrotate-3.18.0.tar.xz

View File

@ -1 +1 @@
10416a3aaea4fbf6c1a01858f2fb994e132c4127 SOURCES/logrotate-3.14.0.tar.xz
6b9aa5efd4551377e9869e8d3303d90a946566f6 SOURCES/logrotate-3.18.0.tar.xz

View File

@ -1,34 +0,0 @@
From b0d067cfba64956893fc095bb37f8c767f5a910e Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Mon, 6 Aug 2018 17:13:31 +0200
Subject: [PATCH] logrotate.8: document the --version option
The man page now covers all the options that are listed
by `logrotate --help`.
Bug: https://bugzilla.redhat.com/1611498
Upstream-commit: 4088ef987df2ec48cc3d968eb87ad27c840fa2d8
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
logrotate.8.in | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/logrotate.8.in b/logrotate.8.in
index 004229e..5ef09c5 100644
--- a/logrotate.8.in
+++ b/logrotate.8.in
@@ -87,6 +87,10 @@ Prints a short usage message.
\fB\-v\fR, \fB\-\-verbose\fR
Turns on verbose mode, for example to display messages during rotation.
+.TP
+\fB\-\-version\fR
+Display version information.
+
.SH CONFIGURATION FILE
\fBlogrotate\fR reads everything about the log files it should be handling
--
2.17.1

View File

@ -0,0 +1,615 @@
From 471cf0a6a90e5d45f116f404e1276ea730dbece6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Fri, 26 Mar 2021 17:18:09 +0100
Subject: [PATCH 1/9] Do not report OOM incorrectly
In case there is no file in the set to rotate `calloc(0, ...)` is called
, which might return NULL.
Order the check for a zero number of files first, to void calling calloc
with a size of zero.
Upstream-commit: 7b65b267d73970eb59061be907c8c35b4396ada9
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
logrotate.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/logrotate.c b/logrotate.c
index 507c85a..a8c8480 100644
--- a/logrotate.c
+++ b/logrotate.c
@@ -2212,11 +2212,6 @@ static int rotateLogSet(const struct logInfo *log, int force)
struct logState **state;
struct logNames **rotNames;
- logHasErrors = calloc(log->numFiles, sizeof(int));
- if (!logHasErrors) {
- message_OOM();
- return 1;
- }
message(MESS_DEBUG, "\nrotating pattern: %s ", log->pattern);
if (force) {
message(MESS_DEBUG, "forced from command line ");
@@ -2277,10 +2272,15 @@ static int rotateLogSet(const struct logInfo *log, int force)
if (log->numFiles == 0) {
message(MESS_DEBUG, "No logs found. Rotation not needed.\n");
- free(logHasErrors);
return 0;
}
+ logHasErrors = calloc(log->numFiles, sizeof(int));
+ if (!logHasErrors) {
+ message_OOM();
+ return 1;
+ }
+
if (log->flags & LOG_FLAG_SU) {
if (switch_user(log->suUid, log->suGid) != 0) {
free(logHasErrors);
--
2.30.2
From 96203f4cdc64e2df3d203231bd1247424a20875e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Mon, 19 Apr 2021 15:35:37 +0200
Subject: [PATCH 2/9] Unify asprintf usage
Unify the error checking of asprintf(3).
Also reset the target string pointer to NULL on error, if it is non-
local, since the content is undefined according to the specification.
Also fix potential NULL-pointer usage in sprintf(3):
logrotate.c:1595:
rotNames->dirName = malloc(strlen(ld) + strlen(log->oldDir) + 2);
sprintf(rotNames->dirName, "%s/%s", ld, log->oldDir);
Upstream-commit: 5afcdeecc5a3bfe07671a3c05c7a301da9206ccd
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 28 +++++++++++++---------------
logrotate.c | 9 ++++++---
2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/config.c b/config.c
index df2d90b..19dcfce 100644
--- a/config.c
+++ b/config.c
@@ -815,21 +815,19 @@ int readAllConfigPaths(const char **paths)
for (i = 0; i < defTabooCount; i++) {
- int bytes;
char *pattern = NULL;
/* generate a pattern by concatenating star (wildcard) to the
* suffix literal
*/
- bytes = asprintf(&pattern, "*%s", defTabooExts[i]);
- if (bytes != -1) {
- tabooPatterns[i] = pattern;
- tabooCount++;
- } else {
+ if (asprintf(&pattern, "*%s", defTabooExts[i]) < 0) {
free_2d_array(tabooPatterns, tabooCount);
message_OOM();
return 1;
}
+
+ tabooPatterns[i] = pattern;
+ tabooCount++;
}
for (file = paths; *file; file++) {
@@ -1421,7 +1419,6 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
}
while (*endtag) {
- int bytes;
char *pattern = NULL;
chptr = endtag;
@@ -1437,10 +1434,11 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
RAISE_ERROR();
}
tabooPatterns = tmp;
- bytes = asprintf(&pattern, "*%.*s", (int)(chptr - endtag), endtag);
+ if (asprintf(&pattern, "*%.*s", (int)(chptr - endtag), endtag) < 0) {
+ message_OOM();
+ RAISE_ERROR();
+ }
- /* should test for malloc() failure */
- assert(bytes != -1);
tabooPatterns[tabooCount] = pattern;
tabooCount++;
}
@@ -1481,7 +1479,6 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
}
while (*endtag) {
- int bytes;
char *pattern = NULL;
char **tmp;
@@ -1496,10 +1493,11 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
RAISE_ERROR();
}
tabooPatterns = tmp;
- bytes = asprintf(&pattern, "%.*s", (int)(chptr - endtag), endtag);
+ if (asprintf(&pattern, "%.*s", (int)(chptr - endtag), endtag) < 0) {
+ message_OOM();
+ RAISE_ERROR();
+ }
- /* should test for malloc() failure */
- assert(bytes != -1);
tabooPatterns[tabooCount] = pattern;
tabooCount++;
@@ -1540,7 +1538,7 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
env_home = pwd->pw_dir;
}
- if (asprintf(&new_key, "%s/%s", env_home, key + 2) == -1) {
+ if (asprintf(&new_key, "%s/%s", env_home, key + 2) < 0) {
message_OOM();
RAISE_ERROR();
}
diff --git a/logrotate.c b/logrotate.c
index a8c8480..e294352 100644
--- a/logrotate.c
+++ b/logrotate.c
@@ -1576,9 +1576,9 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
ld = dirname(logpath);
if (log->oldDir) {
if (log->oldDir[0] != '/') {
- rotNames->dirName =
- malloc(strlen(ld) + strlen(log->oldDir) + 2);
- sprintf(rotNames->dirName, "%s/%s", ld, log->oldDir);
+ if (asprintf(&rotNames->dirName, "%s/%s", ld, log->oldDir) < 0) {
+ rotNames->dirName = NULL;
+ }
} else
rotNames->dirName = strdup(log->oldDir);
} else
@@ -1983,6 +1983,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
if (asprintf(&(rotNames->finalName), "%s/%s%s%s", rotNames->dirName,
rotNames->baseName, dext_str, fileext) < 0) {
message_OOM();
+ rotNames->finalName = NULL;
return 1;
}
if (asprintf(&destFile, "%s%s", rotNames->finalName, compext) < 0) {
@@ -2001,6 +2002,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
if (asprintf(&(rotNames->finalName), "%s/%s.%d%s", rotNames->dirName,
rotNames->baseName, logStart, fileext) < 0) {
message_OOM();
+ rotNames->finalName = NULL;
}
}
@@ -2084,6 +2086,7 @@ static int rotateSingleLog(const struct logInfo *log, unsigned logNum,
free(rotNames->disposeName);
if (asprintf(&rotNames->disposeName, "%s%s", rotNames->finalName, ext) < 0) {
message_OOM();
+ rotNames->disposeName = NULL;
return 1;
}
--
2.30.2
From 3cf921e0d58993b064cd6d52b44835008345f498 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Mon, 19 Apr 2021 15:40:19 +0200
Subject: [PATCH 3/9] Update custom asprintf implementation
Check for vsnprintf(3) failures.
Silence conversion warnings.
Do not call exit(2) on allocation failure, but return -1 like the
specification says. All callers check the return value, since they
need to handle standard asprintf(3) implementations.
Upstream-commit: f917b31dbb47992bf5c5342c7312ddb2e64efc40
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/config.c b/config.c
index 19dcfce..0d79980 100644
--- a/config.c
+++ b/config.c
@@ -61,24 +61,20 @@ int asprintf(char **string_ptr, const char *format, ...)
va_start(arg, format);
size = vsnprintf(NULL, 0, format, arg);
- size++;
va_end(arg);
- va_start(arg, format);
- str = malloc(size);
+ if (size < 0) {
+ return -1;
+ }
+ str = malloc((size_t)size + 1);
if (str == NULL) {
- va_end(arg);
- /*
- * Strictly speaking, GNU asprintf doesn't do this,
- * but the caller isn't checking the return value.
- */
- message_OOM();
- exit(1);
+ return -1;
}
- rv = vsnprintf(str, size, format, arg);
+ va_start(arg, format);
+ rv = vsnprintf(str, (size_t)size + 1, format, arg);
va_end(arg);
*string_ptr = str;
- return (rv);
+ return rv;
}
#endif
--
2.30.2
From ace9818a606a0c96bb6e4da479ed151650b8fa3a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Mon, 19 Apr 2021 15:45:55 +0200
Subject: [PATCH 4/9] Use asprintf instead of split malloc + sprintf
Use asprintf(3) instead of split usage of malloc(3) and sprintf(3) to
reduce the chance of potential size inconsistencies.
Upstream-commit: 001352baa924f021748513b6d09d37eca754d5cc
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 5 ++---
logrotate.c | 25 ++++++++++++-------------
2 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/config.c b/config.c
index 0d79980..2905ff7 100644
--- a/config.c
+++ b/config.c
@@ -1886,13 +1886,12 @@ duperror:
continue;
}
}
- ld = malloc(strlen(dirName) + strlen(newlog->oldDir) + 2);
- if (ld == NULL) {
+ if (asprintf(&ld, "%s/%s", dirName, newlog->oldDir) < 0) {
message_OOM();
free(dirpath);
goto error;
}
- sprintf(ld, "%s/%s", dirName, newlog->oldDir);
+
free(dirpath);
if (newlog->oldDir[0] != '/') {
diff --git a/logrotate.c b/logrotate.c
index e294352..a72329e 100644
--- a/logrotate.c
+++ b/logrotate.c
@@ -1810,15 +1810,6 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
}
}
- /* adding 2 due to / and \0 being added by snprintf */
- rotNames->firstRotated =
- malloc(strlen(rotNames->dirName) + strlen(rotNames->baseName) +
- strlen(fileext) + strlen(compext) + DATEEXT_LEN + 2 );
- if (rotNames->firstRotated == NULL) {
- message_OOM();
- return 1;
- }
-
if (log->flags & LOG_FLAG_DATEEXT) {
/* glob for compressed files with our pattern
* and compress ext */
@@ -1882,9 +1873,13 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
rotNames->disposeName = NULL;
}
/* firstRotated is most recently created/compressed rotated log */
- sprintf(rotNames->firstRotated, "%s/%s%s%s%s",
+ if (asprintf(&rotNames->firstRotated, "%s/%s%s%s%s",
rotNames->dirName, rotNames->baseName, dext_str, fileext,
- (log->flags & LOG_FLAG_DELAYCOMPRESS) ? "" : compext);
+ (log->flags & LOG_FLAG_DELAYCOMPRESS) ? "" : compext) < 0) {
+ message_OOM();
+ rotNames->firstRotated = NULL;
+ return 1;
+ }
globfree(&globResult);
free(glob_pattern);
} else {
@@ -1915,9 +1910,13 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
}
}
- sprintf(rotNames->firstRotated, "%s/%s.%d%s%s", rotNames->dirName,
+ if (asprintf(&rotNames->firstRotated, "%s/%s.%d%s%s", rotNames->dirName,
rotNames->baseName, logStart, fileext,
- (log->flags & LOG_FLAG_DELAYCOMPRESS) ? "" : compext);
+ (log->flags & LOG_FLAG_DELAYCOMPRESS) ? "" : compext) < 0) {
+ message_OOM();
+ rotNames->firstRotated = NULL;
+ return 1;
+ }
for (i = rotateCount + logStart - 1; (i >= 0) && !hasErrors; i--) {
free(newName);
--
2.30.2
From e8a655ef1977add152d79c4dc8148fe7b1c9bca2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Mon, 19 Apr 2021 17:52:48 +0200
Subject: [PATCH 5/9] Mark read-only string variable const
Prevent it accidentally being passed to free(3) or similar.
Upstream-commit: 2231aba823ff6e5a18d996e81ef63df0871224dd
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
logrotate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/logrotate.c b/logrotate.c
index a72329e..7d49261 100644
--- a/logrotate.c
+++ b/logrotate.c
@@ -1567,7 +1567,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
state->lastRotated = now;
{
- char *ld;
+ const char *ld;
char *logpath = strdup(log->files[logNum]);
if (logpath == NULL) {
message_OOM();
--
2.30.2
From c06f20f781c74b2256e8f1757433db7e043b4ddf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Mon, 19 Apr 2021 17:59:21 +0200
Subject: [PATCH 6/9] Limit scope of variable
Limit the scope of a variable, by splitting it into several distinct
block scope variables.
This makes some asprintf(3) calls obsolete, and improves readability by
splitting the purpose of the variable.
Upstream-commit: b37fb75f569b3ddde30dd85184ea160f63abb7d5
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
logrotate.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/logrotate.c b/logrotate.c
index 7d49261..962ac55 100644
--- a/logrotate.c
+++ b/logrotate.c
@@ -1529,7 +1529,6 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
struct logState *state, struct logNames *rotNames)
{
struct tm now;
- char *oldName = NULL;
const char *compext = "";
const char *fileext = "";
int hasErrors = 0;
@@ -1770,11 +1769,8 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
sortGlobResult(&globResult, strlen(rotNames->dirName) + 1 + strlen(rotNames->baseName), dformat);
for (glob_count = 0; glob_count < globResult.gl_pathc && !hasErrors; glob_count++) {
struct stat sbprev;
+ const char *oldName = globResult.gl_pathv[glob_count];
- if (asprintf(&oldName, "%s", (globResult.gl_pathv)[glob_count]) < 0) {
- message_OOM();
- return 1;
- }
if (stat(oldName, &sbprev)) {
if (errno == ENOENT)
message(MESS_DEBUG, "previous log %s does not exist\n", oldName);
@@ -1783,7 +1779,6 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
} else {
hasErrors = compressLogFile(oldName, log, &sbprev);
}
- free(oldName);
}
} else {
message(MESS_DEBUG,
@@ -1793,6 +1788,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
free(glob_pattern);
} else {
struct stat sbprev;
+ char *oldName;
if (asprintf(&oldName, "%s/%s.%d%s", rotNames->dirName,
rotNames->baseName, logStart, fileext) < 0) {
message_OOM();
@@ -1853,16 +1849,14 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
}
if (mail_out != (size_t)-1) {
/* oldName is oldest Backup found (for unlink later) */
- if (asprintf(&oldName, "%s", (globResult.gl_pathv)[mail_out]) < 0) {
- message_OOM();
- return 1;
- }
+ const char *oldName = globResult.gl_pathv[mail_out];
rotNames->disposeName = strdup(oldName);
if (rotNames->disposeName == NULL) {
message_OOM();
+ globfree(&globResult);
+ free(glob_pattern);
return 1;
}
- free(oldName);
} else {
free(rotNames->disposeName);
rotNames->disposeName = NULL;
@@ -1878,6 +1872,8 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
(log->flags & LOG_FLAG_DELAYCOMPRESS) ? "" : compext) < 0) {
message_OOM();
rotNames->firstRotated = NULL;
+ globfree(&globResult);
+ free(glob_pattern);
return 1;
}
globfree(&globResult);
@@ -1885,6 +1881,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
} else {
int i;
char *newName = NULL;
+ char *oldName;
if (rotateCount == -1) {
rotateCount = findLastRotated(rotNames, fileext, compext);
--
2.30.2
From 1a1eb69e6c4ae403edceb411cb0bbc80473e2527 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Tue, 20 Apr 2021 17:41:16 +0200
Subject: [PATCH 7/9] Free memory on noolddir configuration
Consider the following configuration:
olddir /var/log/foo
noolddir
Do not leak the memory of the initial olddir path.
Upstream-commit: 59e8e321d3221a3beaf7b9c99b17d5cb7dbcfaf0
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/config.c b/config.c
index 2905ff7..b7406f5 100644
--- a/config.c
+++ b/config.c
@@ -1134,7 +1134,7 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
if (newlog->dateformat == NULL)
continue;
} else if (!strcmp(key, "noolddir")) {
- newlog->oldDir = NULL;
+ freeLogItem(oldDir);
} else if (!strcmp(key, "mailfirst")) {
newlog->flags |= LOG_FLAG_MAILFIRST;
} else if (!strcmp(key, "maillast")) {
--
2.30.2
From 4aabfd0fe19832ba1df8919356d1d2d4b463937d Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Mon, 3 May 2021 15:14:09 +0200
Subject: [PATCH 8/9] readConfigFile: release `globerr_msg` on all code paths
This eliminates the following reports by Coverity:
Error: RESOURCE_LEAK (CWE-772):
logrotate-3.18.0.18_7a4d/config.c:1798: alloc_arg: "asprintf" allocates memory that is stored into "globerr_msg". [Note: The source code implementation of the function has been overridden by a builtin model.]
logrotate-3.18.0.18_7a4d/config.c:2116: leaked_storage: Variable "globerr_msg" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK (CWE-772):
logrotate-3.18.0.18_7a4d/config.c:1798: alloc_arg: "asprintf" allocates memory that is stored into "globerr_msg". [Note: The source code implementation of the function has been overridden by a builtin model.]
logrotate-3.18.0.18_7a4d/config.c:2122: leaked_storage: Variable "globerr_msg" going out of scope leaks the storage it points to.
Closes: https://github.com/logrotate/logrotate/pull/387
Upstream-commit: 97f841be2bb52b9ac00cd564a6eb0a853d1017bd
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/config.c b/config.c
index b7406f5..91fd412 100644
--- a/config.c
+++ b/config.c
@@ -2086,12 +2086,14 @@ next_state: ;
munmap(buf, length);
close(fd);
+ free(globerr_msg);
return logerror;
error:
/* free is a NULL-safe operation */
free(key);
munmap(buf, length);
close(fd);
+ free(globerr_msg);
return 1;
}
--
2.30.2
From b5610cd1b0bc2cf9ab887007a953fbf6340cb150 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Mon, 3 May 2021 15:17:59 +0200
Subject: [PATCH 9/9] prerotateSingleLog: release `oldName` on all code paths
This eliminates the following reports by Coverity:
Error: RESOURCE_LEAK (CWE-772):
logrotate-3.18.0.18_7a4d/logrotate.c:1911: alloc_arg: "asprintf" allocates memory that is stored into "oldName". [Note: The source code implementation of the function has been overridden by a builtin model.]
logrotate-3.18.0.18_7a4d/logrotate.c:1919: noescape: Resource "oldName" is not freed or pointed-to in "strdup".
logrotate-3.18.0.18_7a4d/logrotate.c:1922: leaked_storage: Variable "oldName" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK (CWE-772):
logrotate-3.18.0.18_7a4d/logrotate.c:1911: alloc_arg: "asprintf" allocates memory that is stored into "oldName". [Note: The source code implementation of the function has been overridden by a builtin model.]
logrotate-3.18.0.18_7a4d/logrotate.c:1919: noescape: Resource "oldName" is not freed or pointed-to in "strdup".
logrotate-3.18.0.18_7a4d/logrotate.c:1931: leaked_storage: Variable "oldName" going out of scope leaks the storage it points to.
Closes: https://github.com/logrotate/logrotate/pull/387
Upstream-commit: 85bc130b19344a3d9c8b472142df14ddcd0a166d
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
logrotate.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/logrotate.c b/logrotate.c
index 962ac55..d3f2825 100644
--- a/logrotate.c
+++ b/logrotate.c
@@ -1903,6 +1903,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
rotNames->disposeName = strdup(oldName);
if (rotNames->disposeName == NULL) {
message_OOM();
+ free(oldName);
return 1;
}
}
@@ -1911,6 +1912,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
rotNames->baseName, logStart, fileext,
(log->flags & LOG_FLAG_DELAYCOMPRESS) ? "" : compext) < 0) {
message_OOM();
+ free(oldName);
rotNames->firstRotated = NULL;
return 1;
}
--
2.30.2

View File

@ -1,630 +0,0 @@
From a4ac21e9a8cfe8a73471a195308a742e07d7fe8d Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Wed, 1 Aug 2018 15:32:38 +0200
Subject: [PATCH 1/3] readConfigFile: assign and check 'key' separately
... to make the code readable. No changes in behavior intended
by this commit.
---
config.c | 312 +++++++++++++++++++++++++++----------------------------
1 file changed, 152 insertions(+), 160 deletions(-)
diff --git a/config.c b/config.c
index 84db36f..d2fba10 100644
--- a/config.c
+++ b/config.c
@@ -1037,7 +1037,8 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
}
if (isalpha((unsigned char)*start)) {
- if ((key = isolateWord(&start, &buf, length)) == NULL)
+ key = isolateWord(&start, &buf, length);
+ if (key == NULL)
continue;
if (!strcmp(key, "compress")) {
newlog->flags |= LOG_FLAG_COMPRESS;
@@ -1191,16 +1192,16 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
}
} else if (!strcmp(key, "shredcycles")) {
free(key);
- if ((key = isolateValue(configFile, lineNum, "shred cycles",
- &start, &buf, length)) != NULL) {
- newlog->shred_cycles = strtoul(key, &chptr, 0);
- if (*chptr || newlog->shred_cycles < 0) {
- message(MESS_ERROR, "%s:%d bad shred cycles '%s'\n",
- configFile, lineNum, key);
- goto error;
- }
+ key = isolateValue(configFile, lineNum, "shred cycles",
+ &start, &buf, length);
+ if (key == NULL)
+ continue;
+ newlog->shred_cycles = strtoul(key, &chptr, 0);
+ if (*chptr || newlog->shred_cycles < 0) {
+ message(MESS_ERROR, "%s:%d bad shred cycles '%s'\n",
+ configFile, lineNum, key);
+ goto error;
}
- else continue;
} else if (!strcmp(key, "hourly")) {
newlog->criterium = ROT_HOURLY;
} else if (!strcmp(key, "daily")) {
@@ -1232,59 +1233,53 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
newlog->criterium = ROT_YEARLY;
} else if (!strcmp(key, "rotate")) {
free(key);
- if ((key = isolateValue
- (configFile, lineNum, "rotate count", &start,
- &buf, length)) != NULL) {
-
- newlog->rotateCount = strtoul(key, &chptr, 0);
- if (*chptr || newlog->rotateCount < 0) {
- message(MESS_ERROR,
- "%s:%d bad rotation count '%s'\n",
- configFile, lineNum, key);
- RAISE_ERROR();
- }
+ key = isolateValue(configFile, lineNum, "rotate count", &start,
+ &buf, length);
+ if (key == NULL)
+ continue;
+ newlog->rotateCount = strtoul(key, &chptr, 0);
+ if (*chptr || newlog->rotateCount < 0) {
+ message(MESS_ERROR,
+ "%s:%d bad rotation count '%s'\n",
+ configFile, lineNum, key);
+ RAISE_ERROR();
}
- else continue;
} else if (!strcmp(key, "start")) {
free(key);
- if ((key = isolateValue
- (configFile, lineNum, "start count", &start,
- &buf, length)) != NULL) {
-
- newlog->logStart = strtoul(key, &chptr, 0);
- if (*chptr || newlog->logStart < 0) {
- message(MESS_ERROR, "%s:%d bad start count '%s'\n",
- configFile, lineNum, key);
- RAISE_ERROR();
- }
+ key = isolateValue(configFile, lineNum, "start count", &start,
+ &buf, length);
+ if (key == NULL)
+ continue;
+ newlog->logStart = strtoul(key, &chptr, 0);
+ if (*chptr || newlog->logStart < 0) {
+ message(MESS_ERROR, "%s:%d bad start count '%s'\n",
+ configFile, lineNum, key);
+ RAISE_ERROR();
}
- else continue;
} else if (!strcmp(key, "minage")) {
free(key);
- if ((key = isolateValue
- (configFile, lineNum, "minage count", &start,
- &buf, length)) != NULL) {
- newlog->rotateMinAge = strtoul(key, &chptr, 0);
- if (*chptr || newlog->rotateMinAge < 0) {
- message(MESS_ERROR, "%s:%d bad minimum age '%s'\n",
- configFile, lineNum, start);
- RAISE_ERROR();
- }
+ key = isolateValue(configFile, lineNum, "minage count", &start,
+ &buf, length);
+ if (key == NULL)
+ continue;
+ newlog->rotateMinAge = strtoul(key, &chptr, 0);
+ if (*chptr || newlog->rotateMinAge < 0) {
+ message(MESS_ERROR, "%s:%d bad minimum age '%s'\n",
+ configFile, lineNum, start);
+ RAISE_ERROR();
}
- else continue;
} else if (!strcmp(key, "maxage")) {
free(key);
- if ((key = isolateValue
- (configFile, lineNum, "maxage count", &start,
- &buf, length)) != NULL) {
- newlog->rotateAge = strtoul(key, &chptr, 0);
- if (*chptr || newlog->rotateAge < 0) {
- message(MESS_ERROR, "%s:%d bad maximum age '%s'\n",
- configFile, lineNum, start);
- RAISE_ERROR();
- }
+ key = isolateValue(configFile, lineNum, "maxage count", &start,
+ &buf, length);
+ if (key == NULL)
+ continue;
+ newlog->rotateAge = strtoul(key, &chptr, 0);
+ if (*chptr || newlog->rotateAge < 0) {
+ message(MESS_ERROR, "%s:%d bad maximum age '%s'\n",
+ configFile, lineNum, start);
+ RAISE_ERROR();
}
- else continue;
} else if (!strcmp(key, "errors")) {
message(MESS_DEBUG,
"%s: %d: the errors directive is deprecated and no longer used.\n",
@@ -1337,48 +1332,48 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
continue;
}
free(key);
- if ((key = isolateValue(configFile, lineNum, "tabooext", &start,
- &buf, length)) != NULL) {
- endtag = key;
- if (*endtag == '+') {
+ key = isolateValue(configFile, lineNum, "tabooext", &start,
+ &buf, length);
+ if (key == NULL)
+ continue;
+ endtag = key;
+ if (*endtag == '+') {
+ endtag++;
+ while (isspace((unsigned char)*endtag) && *endtag)
endtag++;
- while (isspace((unsigned char)*endtag) && *endtag)
- endtag++;
- } else {
- free_2d_array(tabooPatterns, tabooCount);
- tabooCount = 0;
- /* realloc of NULL is safe by definition */
- tabooPatterns = NULL;
- }
-
- while (*endtag) {
- int bytes;
- char *pattern = NULL;
+ } else {
+ free_2d_array(tabooPatterns, tabooCount);
+ tabooCount = 0;
+ /* realloc of NULL is safe by definition */
+ tabooPatterns = NULL;
+ }
- chptr = endtag;
- while (!isspace((unsigned char)*chptr) && *chptr != ',' && *chptr)
- chptr++;
+ while (*endtag) {
+ int bytes;
+ char *pattern = NULL;
- /* accept only non-empty patterns to avoid exclusion of everything */
- if (endtag < chptr) {
- tabooPatterns = realloc(tabooPatterns, sizeof(*tabooPatterns) *
- (tabooCount + 1));
- bytes = asprintf(&pattern, "*%.*s", (int)(chptr - endtag), endtag);
+ chptr = endtag;
+ while (!isspace((unsigned char)*chptr) && *chptr != ',' && *chptr)
+ chptr++;
- /* should test for malloc() failure */
- assert(bytes != -1);
- tabooPatterns[tabooCount] = pattern;
- tabooCount++;
- }
+ /* accept only non-empty patterns to avoid exclusion of everything */
+ if (endtag < chptr) {
+ tabooPatterns = realloc(tabooPatterns, sizeof(*tabooPatterns) *
+ (tabooCount + 1));
+ bytes = asprintf(&pattern, "*%.*s", (int)(chptr - endtag), endtag);
- endtag = chptr;
- if (*endtag == ',')
- endtag++;
- while (*endtag && isspace((unsigned char)*endtag))
- endtag++;
+ /* should test for malloc() failure */
+ assert(bytes != -1);
+ tabooPatterns[tabooCount] = pattern;
+ tabooCount++;
}
+
+ endtag = chptr;
+ if (*endtag == ',')
+ endtag++;
+ while (*endtag && isspace((unsigned char)*endtag))
+ endtag++;
}
- else continue;
} else if (!strcmp(key, "taboopat")) {
if (newlog != defConfig) {
message(MESS_ERROR,
@@ -1389,68 +1384,68 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
continue;
}
free(key);
- if ((key = isolateValue(configFile, lineNum, "taboopat", &start,
- &buf, length)) != NULL) {
- endtag = key;
- if (*endtag == '+') {
+ key = isolateValue(configFile, lineNum, "taboopat", &start,
+ &buf, length);
+ if (key == NULL)
+ continue;
+
+ endtag = key;
+ if (*endtag == '+') {
+ endtag++;
+ while (isspace((unsigned char)*endtag) && *endtag)
endtag++;
- while (isspace((unsigned char)*endtag) && *endtag)
- endtag++;
- } else {
- free_2d_array(tabooPatterns, tabooCount);
- tabooCount = 0;
- /* realloc of NULL is safe by definition */
- tabooPatterns = NULL;
- }
+ } else {
+ free_2d_array(tabooPatterns, tabooCount);
+ tabooCount = 0;
+ /* realloc of NULL is safe by definition */
+ tabooPatterns = NULL;
+ }
- while (*endtag) {
- int bytes;
- char *pattern = NULL;
+ while (*endtag) {
+ int bytes;
+ char *pattern = NULL;
- chptr = endtag;
- while (!isspace((unsigned char)*chptr) && *chptr != ',' && *chptr)
- chptr++;
+ chptr = endtag;
+ while (!isspace((unsigned char)*chptr) && *chptr != ',' && *chptr)
+ chptr++;
- tabooPatterns = realloc(tabooPatterns, sizeof(*tabooPatterns) *
- (tabooCount + 1));
- bytes = asprintf(&pattern, "%.*s", (int)(chptr - endtag), endtag);
+ tabooPatterns = realloc(tabooPatterns, sizeof(*tabooPatterns) *
+ (tabooCount + 1));
+ bytes = asprintf(&pattern, "%.*s", (int)(chptr - endtag), endtag);
- /* should test for malloc() failure */
- assert(bytes != -1);
- tabooPatterns[tabooCount] = pattern;
- tabooCount++;
+ /* should test for malloc() failure */
+ assert(bytes != -1);
+ tabooPatterns[tabooCount] = pattern;
+ tabooCount++;
- endtag = chptr;
- if (*endtag == ',')
- endtag++;
- while (*endtag && isspace((unsigned char)*endtag))
- endtag++;
- }
+ endtag = chptr;
+ if (*endtag == ',')
+ endtag++;
+ while (*endtag && isspace((unsigned char)*endtag))
+ endtag++;
}
- else continue;
} else if (!strcmp(key, "include")) {
free(key);
- if ((key = isolateValue(configFile, lineNum, "include", &start,
- &buf, length)) != NULL) {
-
- message(MESS_DEBUG, "including %s\n", key);
- if (recursion_depth >= MAX_NESTING) {
- message(MESS_ERROR, "%s:%d include nesting too deep\n",
- configFile, lineNum);
- logerror = 1;
- continue;
- }
+ key = isolateValue(configFile, lineNum, "include", &start,
+ &buf, length);
+ if (key == NULL)
+ continue;
+ message(MESS_DEBUG, "including %s\n", key);
+ if (recursion_depth >= MAX_NESTING) {
+ message(MESS_ERROR, "%s:%d include nesting too deep\n",
+ configFile, lineNum);
+ logerror = 1;
+ continue;
+ }
- ++recursion_depth;
- rv = readConfigPath(key, newlog);
- --recursion_depth;
+ ++recursion_depth;
+ rv = readConfigPath(key, newlog);
+ --recursion_depth;
- if (rv) {
- logerror = 1;
- continue;
- }
+ if (rv) {
+ logerror = 1;
+ continue;
}
- else continue;
} else if (!strcmp(key, "olddir")) {
freeLogItem (oldDir);
@@ -1460,28 +1455,23 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
}
message(MESS_DEBUG, "olddir is now %s\n", newlog->oldDir);
} else if (!strcmp(key, "extension")) {
- if ((key = isolateValue
- (configFile, lineNum, "extension name", &start,
- &buf, length)) != NULL) {
- freeLogItem (extension);
- newlog->extension = key;
- key = NULL;
- }
- else continue;
-
- message(MESS_DEBUG, "extension is now %s\n",
- newlog->extension);
+ key = isolateValue(configFile, lineNum, "extension name", &start,
+ &buf, length);
+ if (key == NULL)
+ continue;
+ freeLogItem (extension);
+ newlog->extension = key;
+ key = NULL;
+ message(MESS_DEBUG, "extension is now %s\n", newlog->extension);
} else if (!strcmp(key, "addextension")) {
- if ((key = isolateValue
- (configFile, lineNum, "addextension name", &start,
- &buf, length)) != NULL) {
- freeLogItem (addextension);
- newlog->addextension = key;
- key = NULL;
- }
- else continue;
-
+ key = isolateValue(configFile, lineNum, "addextension name", &start,
+ &buf, length);
+ if (key == NULL)
+ continue;
+ freeLogItem (addextension);
+ newlog->addextension = key;
+ key = NULL;
message(MESS_DEBUG, "addextension is now %s\n",
newlog->addextension);
@@ -1827,7 +1817,8 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
break;
case STATE_LOAD_SCRIPT:
case STATE_LOAD_SCRIPT | STATE_SKIP_CONFIG:
- if ((key = isolateWord(&start, &buf, length)) == NULL)
+ key = isolateWord(&start, &buf, length);
+ if (key == NULL)
continue;
if (strcmp(key, "endscript") == 0) {
@@ -1862,7 +1853,8 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
newlog = defConfig;
}
else {
- if ((key = isolateWord(&start, &buf, length)) == NULL)
+ key = isolateWord(&start, &buf, length);
+ if (key == NULL)
continue;
if (
(strcmp(key, "postrotate") == 0) ||
--
2.17.1
From a3a955494999bd4861f14b846c345cbc96715262 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Wed, 1 Aug 2018 15:09:40 +0200
Subject: [PATCH 2/3] readConfigFile: assign and free 'key' consistently
This commit fixes the following memory leaks (detected by Coverity):
Error: RESOURCE_LEAK:
config.c:1466: overwrite_var: Overwriting "key" in "key = isolateValue(configFile, lineNum, "extension name", &start, &buf, length)" leaks the storage that "key" points to.
Error: RESOURCE_LEAK:
config.c:1479: overwrite_var: Overwriting "key" in "key = isolateValue(configFile, lineNum, "addextension name", &start, &buf, length)" leaks the storage that "key" points to.
Error: RESOURCE_LEAK:
config.c:1043: alloc_fn: Storage is returned from allocation function "isolateWord".
config.c:219:2: alloc_fn: Storage is returned from allocation function "strndup".
config.c:219:2: assign: Assigning: "key" = "strndup(start, endtag - start)".
config.c:221:2: return_alloc: Returning allocated memory "key".
config.c:1043: var_assign: Assigning: "key" = storage returned from "isolateWord(&start, &buf, length)".
config.c:1928: leaked_storage: Variable "key" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK:
config.c:1153: alloc_fn: Storage is returned from allocation function "isolateValue".
config.c:204:2: alloc_fn: Storage is returned from allocation function "isolateLine".
config.c:178:2: alloc_fn: Storage is returned from allocation function "strndup".
config.c:178:2: assign: Assigning: "key" = "strndup(start, endtag - start + 1L)".
config.c:180:2: return_alloc: Returning allocated memory "key".
config.c:204:2: return_alloc_fn: Directly returning storage allocated by "isolateLine".
config.c:1153: var_assign: Assigning: "key" = storage returned from "isolateValue(configFile, lineNum, opt, &start, &buf, length)".
config.c:1928: leaked_storage: Variable "key" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK:
config.c:1219: alloc_fn: Storage is returned from allocation function "isolateLine".
config.c:178:2: alloc_fn: Storage is returned from allocation function "strndup".
config.c:178:2: assign: Assigning: "key" = "strndup(start, endtag - start + 1L)".
config.c:180:2: return_alloc: Returning allocated memory "key".
config.c:1219: var_assign: Assigning: "key" = storage returned from "isolateLine(&start, &buf, length)".
config.c:1928: leaked_storage: Variable "key" going out of scope leaks the storage it points to.
Closes #208
---
config.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/config.c b/config.c
index d2fba10..39c9bc7 100644
--- a/config.c
+++ b/config.c
@@ -1022,10 +1022,6 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
start = buf;
for (start = buf; start - buf < length; start++) {
- if (key) {
- free(key);
- key = NULL;
- }
switch (state) {
case STATE_DEFAULT:
if (isblank((unsigned char)*start))
@@ -1037,6 +1033,7 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
}
if (isalpha((unsigned char)*start)) {
+ free(key);
key = isolateWord(&start, &buf, length);
if (key == NULL)
continue;
@@ -1455,6 +1452,7 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
}
message(MESS_DEBUG, "olddir is now %s\n", newlog->oldDir);
} else if (!strcmp(key, "extension")) {
+ free(key);
key = isolateValue(configFile, lineNum, "extension name", &start,
&buf, length);
if (key == NULL)
@@ -1465,6 +1463,7 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
message(MESS_DEBUG, "extension is now %s\n", newlog->extension);
} else if (!strcmp(key, "addextension")) {
+ free(key);
key = isolateValue(configFile, lineNum, "addextension name", &start,
&buf, length);
if (key == NULL)
@@ -1557,8 +1556,6 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
if (*start != '\n')
state = STATE_SKIP_LINE;
}
- free(key);
- key = NULL;
} else if (*start == '/' || *start == '"' || *start == '\''
#ifdef GLOB_TILDE
|| *start == '~'
@@ -1817,6 +1814,7 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
break;
case STATE_LOAD_SCRIPT:
case STATE_LOAD_SCRIPT | STATE_SKIP_CONFIG:
+ free(key);
key = isolateWord(&start, &buf, length);
if (key == NULL)
continue;
@@ -1853,6 +1851,7 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
newlog = defConfig;
}
else {
+ free(key);
key = isolateWord(&start, &buf, length);
if (key == NULL)
continue;
@@ -1884,8 +1883,6 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
state = STATE_SKIP_LINE | STATE_SKIP_CONFIG;
}
}
- free(key);
- key = NULL;
}
break;
default:
@@ -1893,10 +1890,6 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
"%s: %d: readConfigFile() unknown state\n",
configFile, lineNum);
}
- if (key) {
- free(key);
- key = NULL;
- }
if (*start == '\n') {
lineNum++;
}
@@ -1910,6 +1903,8 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
goto error;
}
+ free(key);
+
munmap(buf, (size_t) length);
close(fd);
return logerror;
--
2.17.1
From 771af94fd6c6299a7cb3d20c8b247591775653d3 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Wed, 1 Aug 2018 16:06:27 +0200
Subject: [PATCH 3/3] simplify code of prerotateSingleLog()
... to eliminate a use-after-free false positive reported by Coverity:
Error: USE_AFTER_FREE:
logrotate.c:1800: freed_arg: "free" frees "oldName".
logrotate.c:1779: use_after_free: Using freed pointer "oldName".
Closes #209
---
logrotate.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/logrotate.c b/logrotate.c
index 02d45e9..95fd70b 100644
--- a/logrotate.c
+++ b/logrotate.c
@@ -1353,7 +1353,7 @@ static int prerotateSingleLog(struct logInfo *log, int logNum,
struct logState *state, struct logNames *rotNames)
{
struct tm now = *localtime(&nowSecs);
- char *oldName, *newName = NULL;
+ char *oldName = NULL;
const char *compext = "";
const char *fileext = "";
int hasErrors = 0;
@@ -1670,6 +1670,7 @@ static int prerotateSingleLog(struct logInfo *log, int logNum,
free(glob_pattern);
} else {
int i;
+ char *newName = NULL;
if (log->rotateAge) {
struct stat fst_buf;
for (i = 1; i <= rotateCount + 1; i++) {
@@ -1697,7 +1698,6 @@ static int prerotateSingleLog(struct logInfo *log, int logNum,
compext) < 0) {
message(MESS_FATAL, "could not allocate disposeName memory\n");
}
- newName = strdup(oldName);
rotNames->disposeName = strdup(oldName);
@@ -1711,6 +1711,8 @@ static int prerotateSingleLog(struct logInfo *log, int logNum,
if (asprintf(&oldName, "%s/%s.%d%s%s", rotNames->dirName,
rotNames->baseName, i, fileext, compext) < 0) {
message(MESS_FATAL, "could not allocate oldName memory\n");
+ oldName = NULL;
+ break;
}
message(MESS_DEBUG,
@@ -1727,11 +1729,9 @@ static int prerotateSingleLog(struct logInfo *log, int logNum,
hasErrors = 1;
}
}
- if (hasErrors || i - 1 < 0)
- free(oldName);
-
}
free(newName);
+ free(oldName);
} /* !LOG_FLAG_DATEEXT */
if (log->flags & LOG_FLAG_DATEEXT) {
--
2.17.1

View File

@ -0,0 +1,62 @@
From 4810afca1223099c1546da8d73d653c0d1eff96e Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Tue, 27 Apr 2021 18:36:30 +0200
Subject: [PATCH 1/2] logrotate.8: unify documentation of
copy/copytruncate/renamecopy
Bug: https://bugzilla.redhat.com/1934629
Closes: https://github.com/logrotate/logrotate/pull/386
Upstream-commit: 6ac9fe5759678b4c2b312eea490ebbae25092213
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
logrotate.8.in | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/logrotate.8.in b/logrotate.8.in
index f27c279..8064d68 100644
--- a/logrotate.8.in
+++ b/logrotate.8.in
@@ -411,7 +411,8 @@ Make a copy of the log file, but don't change the original at all.
This option can be used, for instance, to make a snapshot of the current
log file, or when some other utility needs to truncate or parse the file.
When this option is used, the \fBcreate\fR option will have no effect,
-as the old log file stays in place.
+as the old log file stays in place. The \fBcopy\fR option allows storing
+rotated log files on the different devices using \fBolddir\fR directive.
.TP
\fBnocopy\fR
@@ -427,7 +428,9 @@ and thus might continue writing (appending) to the previous log file forever.
Note that there is a very small time slice between copying the file and
truncating it, so some logging data might be lost.
When this option is used, the \fBcreate\fR option will have no effect,
-as the old log file stays in place.
+as the old log file stays in place. The \fBcopytruncate\fR option allows
+storing rotated log files on the different devices using \fBolddir\fR
+directive.
.TP
\fBnocopytruncate\fR
@@ -438,9 +441,14 @@ Do not truncate the original log file in place after creating a copy
\fBrenamecopy\fR
Log file is renamed to temporary filename in the same directory by adding
".tmp" extension to it. After that, \fBpostrotate\fR script is run
-and log file is copied from temporary filename to final filename. This allows
-storing rotated log files on the different devices using \fBolddir\fR
-directive. In the end, temporary filename is removed.
+and log file is copied from temporary filename to final filename. In the end,
+temporary filename is removed. The \fBrenamecopy\fR option allows storing
+rotated log files on the different devices using \fBolddir\fR directive.
+
+.TP
+\fBnorenamecopy\fR
+Do not rename and copy the original log file
+(this overrides the \fBrenamecopy\fR option).
.TP
\fBshred\fR
--
2.30.2

View File

@ -1,89 +0,0 @@
From b98dd1933b1ebf5c86041bf135af421fe1ce4fc9 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Fri, 28 Jun 2019 18:22:39 +0200
Subject: [PATCH] globerr: do not abort globbing on broken symlink
Fixes #251
Upstream-commit: 4297f01103915f4ee356d37bdb35e8c41bbbdb28
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 16 +++++++++++++---
test/Makefile.am | 1 +
test/test-0084.sh | 14 ++++++++++++++
test/test-config.84.in | 3 +++
4 files changed, 31 insertions(+), 3 deletions(-)
create mode 100755 test/test-0084.sh
create mode 100644 test/test-config.84.in
diff --git a/config.c b/config.c
index e4807c9..1805a16 100644
--- a/config.c
+++ b/config.c
@@ -834,9 +834,19 @@ static int globerr(const char *pathname, int theerr)
{
(void) pathname;
- /* A missing directory is not an error, so return 0 */
- if (theerr == ENOTDIR)
- return 0;
+ /* prevent glob() from being aborted in certain cases */
+ switch (theerr) {
+ case ENOTDIR:
+ /* non-directory where directory was expected by the glob */
+ return 0;
+
+ case ENOENT:
+ /* most likely symlink with non-existent target */
+ return 0;
+
+ default:
+ break;
+ }
glob_errno = theerr;
diff --git a/test/Makefile.am b/test/Makefile.am
index 5e838d1..35ba2b9 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -76,6 +76,7 @@ TEST_CASES = \
test-0075.sh \
test-0076.sh \
test-0077.sh \
+ test-0084.sh \
test-0100.sh \
test-0101.sh
diff --git a/test/test-0084.sh b/test/test-0084.sh
new file mode 100755
index 0000000..1389331
--- /dev/null
+++ b/test/test-0084.sh
@@ -0,0 +1,14 @@
+#!/bin/bash
+
+. ./test-common.sh
+
+cleanup 84
+
+# ------------------------------- Test 84 ------------------------------------
+preptest test.log 84 1
+
+mkdir -p log/dir
+ln -s XXX log/sym
+touch log/dir/file
+
+$RLR test-config.84 -v --force
diff --git a/test/test-config.84.in b/test/test-config.84.in
new file mode 100644
index 0000000..1a79bfe
--- /dev/null
+++ b/test/test-config.84.in
@@ -0,0 +1,3 @@
+&DIR&/log/*/* {
+ rotate 1
+}
--
2.21.3

View File

@ -0,0 +1,89 @@
From 337eb1492f8b694542d704c7a4612e3211f717e5 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Tue, 27 Apr 2021 20:52:32 +0200
Subject: [PATCH 2/2] make `renamecopy` and `copytruncate` override each other
These option cannot work together. This rule prevents unnecessary
rotation failure in case one of the options comes from the global
configuration and the other one from log-specific configuration.
Bug: https://bugzilla.redhat.com/1934601
Closes: https://github.com/logrotate/logrotate/pull/386
Upstream-commit: fe53a0efd21c11dbe9705564f92f5d9aa6bf855e
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 2 ++
logrotate.8.in | 3 ++-
test/test-config.24.in | 3 +++
test/test-config.58.in | 3 +++
4 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/config.c b/config.c
index 91fd412..1bca9e4 100644
--- a/config.c
+++ b/config.c
@@ -1106,10 +1106,12 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
newlog->flags &= ~LOG_FLAG_SHAREDSCRIPTS;
} else if (!strcmp(key, "copytruncate")) {
newlog->flags |= LOG_FLAG_COPYTRUNCATE;
+ newlog->flags &= ~LOG_FLAG_TMPFILENAME;
} else if (!strcmp(key, "nocopytruncate")) {
newlog->flags &= ~LOG_FLAG_COPYTRUNCATE;
} else if (!strcmp(key, "renamecopy")) {
newlog->flags |= LOG_FLAG_TMPFILENAME;
+ newlog->flags &= ~LOG_FLAG_COPYTRUNCATE;
} else if (!strcmp(key, "norenamecopy")) {
newlog->flags &= ~LOG_FLAG_TMPFILENAME;
} else if (!strcmp(key, "copy")) {
diff --git a/logrotate.8.in b/logrotate.8.in
index 8064d68..f0aa23f 100644
--- a/logrotate.8.in
+++ b/logrotate.8.in
@@ -430,7 +430,7 @@ truncating it, so some logging data might be lost.
When this option is used, the \fBcreate\fR option will have no effect,
as the old log file stays in place. The \fBcopytruncate\fR option allows
storing rotated log files on the different devices using \fBolddir\fR
-directive.
+directive. The \fBcopytruncate\fR option implies \fBnorenamecopy\fR.
.TP
\fBnocopytruncate\fR
@@ -444,6 +444,7 @@ Log file is renamed to temporary filename in the same directory by adding
and log file is copied from temporary filename to final filename. In the end,
temporary filename is removed. The \fBrenamecopy\fR option allows storing
rotated log files on the different devices using \fBolddir\fR directive.
+The \fBrenamecopy\fR option implies \fBnocopytruncate\fR.
.TP
\fBnorenamecopy\fR
diff --git a/test/test-config.24.in b/test/test-config.24.in
index 35cfcd3..7a2a760 100644
--- a/test/test-config.24.in
+++ b/test/test-config.24.in
@@ -1,5 +1,8 @@
create
+# will be overridden by copytruncate
+renamecopy
+
&DIR&/test*.log {
daily
copytruncate
diff --git a/test/test-config.58.in b/test/test-config.58.in
index 34906da..79058be 100644
--- a/test/test-config.58.in
+++ b/test/test-config.58.in
@@ -1,5 +1,8 @@
create
+# will be overridden by renamecopy
+copytruncate
+
&DIR&/test.log {
renamecopy
weekly
--
2.30.2

View File

@ -1,37 +0,0 @@
From 893ab396daffebfe5bb97e9fcf0adbd7fda1b828 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Fri, 18 Jan 2019 16:10:56 +0100
Subject: [PATCH] logrotate.8: encourage admins to use the `su` directive
... to rotate files in directories that are directly or indirectly in
control of non-privileged users. Originally reported in the following
pull request:
https://github.com/logrotate/logrotate/pull/235
Closes #236
Upstream-commit: 3e170c0609a18e0bb5fd7f647cb877221d576456
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
logrotate.8.in | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/logrotate.8.in b/logrotate.8.in
index 56c4a32..ee26821 100644
--- a/logrotate.8.in
+++ b/logrotate.8.in
@@ -575,7 +575,9 @@ user/group (usually root). \fIuser\fR specifies the user name used for
rotation and \fIgroup\fR specifies the group used for rotation. If the
user/group you specify here does not have sufficient privilege to make
files with the ownership you've specified in a \fIcreate\fR instruction,
-it will cause an error.
+it will cause an error. If logrotate runs with root privileges, it is
+recommended to use the \fBsu\fR directive to rotate files in directories
+that are directly or indirectly in control of non-privileged users.
.TP
\fBtabooext\fR [+] \fIlist\fR
--
2.21.3

View File

@ -0,0 +1,253 @@
From 53e0dc4a8ddcb169b0ba36472de03f4366f45159 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Tue, 29 Mar 2022 21:06:54 +0200
Subject: [PATCH 1/3] skip locking if state file is world-readable
Fixes: CVE-2022-1348 - potential DoS from unprivileged users via the state file
Bug: https://bugzilla.redhat.com/CVE-2022-1348
Upstream-commit: 1f76a381e2caa0603ae3dbc51ed0f1aa0d6658b9
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
logrotate.c | 24 ++++++++++++++++++++++--
logrotate.spec.in | 3 +--
test/Makefile.am | 1 +
test/test-0087.sh | 1 +
test/test-0092.sh | 19 +++++++++++++++++++
test/test-config.92.in | 4 ++++
6 files changed, 48 insertions(+), 4 deletions(-)
create mode 100755 test/test-0092.sh
create mode 100644 test/test-config.92.in
diff --git a/logrotate.c b/logrotate.c
index d3f2825..78153b3 100644
--- a/logrotate.c
+++ b/logrotate.c
@@ -2565,6 +2565,9 @@ static int writeState(const char *stateFilename)
close(fdcurr);
+ /* drop world-readable flag to prevent others from locking */
+ sb.st_mode &= ~(mode_t)S_IROTH;
+
fdsave = createOutputFile(tmpFilename, O_RDWR | O_CREAT | O_TRUNC, &sb, prev_acl, 0);
#ifdef WITH_ACL
if (prev_acl) {
@@ -2898,15 +2901,16 @@ static int readState(const char *stateFilename)
static int lockState(const char *stateFilename, int skip_state_lock)
{
+ struct stat sb;
int lockFd = open(stateFilename, O_RDWR | O_CLOEXEC);
if (lockFd == -1) {
if (errno == ENOENT) {
message(MESS_DEBUG, "Creating stub state file: %s\n",
stateFilename);
- /* create a stub state file with mode 0644 */
+ /* create a stub state file with mode 0640 */
lockFd = open(stateFilename, O_CREAT | O_EXCL | O_WRONLY,
- S_IWUSR | S_IRUSR | S_IRGRP | S_IROTH);
+ S_IWUSR | S_IRUSR | S_IRGRP);
if (lockFd == -1) {
message(MESS_ERROR, "error creating stub state file %s: %s\n",
stateFilename, strerror(errno));
@@ -2926,6 +2930,22 @@ static int lockState(const char *stateFilename, int skip_state_lock)
return 0;
}
+ if (fstat(lockFd, &sb) == -1) {
+ message(MESS_ERROR, "error stat()ing state file %s: %s\n",
+ stateFilename, strerror(errno));
+ close(lockFd);
+ return 1;
+ }
+
+ if (sb.st_mode & S_IROTH) {
+ message(MESS_ERROR, "state file %s is world-readable and thus can"
+ " be locked from other unprivileged users."
+ " Skipping lock acquisition...\n",
+ stateFilename);
+ close(lockFd);
+ return 0;
+ }
+
if (flock(lockFd, LOCK_EX | LOCK_NB) == -1) {
if (errno == EWOULDBLOCK) {
message(MESS_ERROR, "state file %s is already locked\n"
diff --git a/logrotate.spec.in b/logrotate.spec.in
index 92e1d97..3caabf2 100644
--- a/logrotate.spec.in
+++ b/logrotate.spec.in
@@ -41,7 +41,6 @@ install -p -m 644 examples/logrotate.conf $RPM_BUILD_ROOT%{_sysconfdir}/logrotat
install -p -m 644 examples/btmp $RPM_BUILD_ROOT%{_sysconfdir}/logrotate.d/btmp
install -p -m 644 examples/wtmp $RPM_BUILD_ROOT%{_sysconfdir}/logrotate.d/wtmp
install -p -m 755 examples/logrotate.cron $RPM_BUILD_ROOT%{_sysconfdir}/cron.daily/logrotate
-touch $RPM_BUILD_ROOT%{_localstatedir}/lib/logrotate.status
%clean
rm -rf $RPM_BUILD_ROOT
@@ -55,4 +54,4 @@ rm -rf $RPM_BUILD_ROOT
%attr(0755, root, root) %{_sysconfdir}/cron.daily/logrotate
%attr(0644, root, root) %config(noreplace) %{_sysconfdir}/logrotate.conf
%attr(0755, root, root) %{_sysconfdir}/logrotate.d
-%attr(0644, root, root) %verify(not size md5 mtime) %config(noreplace) %{_localstatedir}/lib/logrotate.status
+%ghost %attr(0640, root, root) %verify(not size md5 mtime) %{_localstatedir}/lib/logrotate.status
diff --git a/test/Makefile.am b/test/Makefile.am
index 914fe65..d6fb7c8 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -87,6 +87,7 @@ TEST_CASES = \
test-0086.sh \
test-0087.sh \
test-0088.sh \
+ test-0092.sh \
test-0100.sh \
test-0101.sh
diff --git a/test/test-0087.sh b/test/test-0087.sh
index 91e5266..aeff2c6 100755
--- a/test/test-0087.sh
+++ b/test/test-0087.sh
@@ -8,6 +8,7 @@ cleanup 87
preptest test.log 87 1
touch state
+chmod 0640 state
$RLR test-config.87 -f &
diff --git a/test/test-0092.sh b/test/test-0092.sh
new file mode 100755
index 0000000..be52e14
--- /dev/null
+++ b/test/test-0092.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+. ./test-common.sh
+
+# check state file locking
+cleanup 92
+
+preptest test.log 92 1
+
+touch state
+chmod 0644 state
+flock state -c "sleep 10" &
+
+$RLR -f test-config.92 || exit 23
+
+checkoutput <<EOF
+test.log 0
+test.log.1 0 zero
+EOF
diff --git a/test/test-config.92.in b/test/test-config.92.in
new file mode 100644
index 0000000..ac93900
--- /dev/null
+++ b/test/test-config.92.in
@@ -0,0 +1,4 @@
+&DIR&/test.log {
+ rotate 1
+ create
+}
--
2.35.3
From 0d2d770cc5aa7bf14e84a2832249eeeb391b0b8a Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Wed, 25 May 2022 09:55:02 +0200
Subject: [PATCH 2/3] drop world-readable permission on state file
... even when ACLs are enabled. This is a follow-up to the fix
of CVE-2022-1348. It has no impact on security but makes the state
file locking work again in more cases.
Closes: https://github.com/logrotate/logrotate/pull/446
Upstream-commit: addbd293242b0b78aa54f054e6c1d249451f137d
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
logrotate.c | 10 +++++++---
test/test-0048.sh | 1 +
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/logrotate.c b/logrotate.c
index 78153b3..8d49f26 100644
--- a/logrotate.c
+++ b/logrotate.c
@@ -2498,6 +2498,7 @@ static int writeState(const char *stateFilename)
struct tm now;
time_t now_time, last_time;
char *prevCtx;
+ int force_mode = 0;
localtime_r(&nowSecs, &now);
@@ -2565,10 +2566,13 @@ static int writeState(const char *stateFilename)
close(fdcurr);
- /* drop world-readable flag to prevent others from locking */
- sb.st_mode &= ~(mode_t)S_IROTH;
+ if (sb.st_mode & (mode_t)S_IROTH) {
+ /* drop world-readable flag to prevent others from locking */
+ sb.st_mode &= ~(mode_t)S_IROTH;
+ force_mode = 1;
+ }
- fdsave = createOutputFile(tmpFilename, O_RDWR | O_CREAT | O_TRUNC, &sb, prev_acl, 0);
+ fdsave = createOutputFile(tmpFilename, O_RDWR | O_CREAT | O_TRUNC, &sb, prev_acl, force_mode);
#ifdef WITH_ACL
if (prev_acl) {
acl_free(prev_acl);
diff --git a/test/test-0048.sh b/test/test-0048.sh
index 62d606b..06b255a 100755
--- a/test/test-0048.sh
+++ b/test/test-0048.sh
@@ -18,6 +18,7 @@ cat > state << EOF
logrotate state -- version 2
EOF
+chmod 0640 state
setfacl -m u:nobody:rwx state
$RLR test-config.48
--
2.35.3
From 105ed9f433a3aaf1aec93318aa9c8811b59d7b23 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Fri, 27 May 2022 09:56:07 +0200
Subject: [PATCH 3/3] lockState: do not print `error:` when exit code is
unaffected
Closes: https://github.com/logrotate/logrotate/pull/448
Upstream-commit: 31cf1099ab8514dfcae5a980bc77352edd5292f8
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
logrotate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/logrotate.c b/logrotate.c
index 27deaf3..77db8c2 100644
--- a/logrotate.c
+++ b/logrotate.c
@@ -2942,8 +2942,8 @@ static int lockState(const char *stateFilename, int skip_state_lock)
}
if (sb.st_mode & S_IROTH) {
- message(MESS_ERROR, "state file %s is world-readable and thus can"
- " be locked from other unprivileged users."
+ message(MESS_NORMAL, "warning: state file %s is world-readable"
+ " and thus can be locked from other unprivileged users."
" Skipping lock acquisition...\n",
stateFilename);
close(lockFd);
--
2.35.3

View File

@ -1,33 +0,0 @@
From a045dbad7370109a8ddf16a24090b8357a9b73fd Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Mon, 26 Aug 2019 15:13:16 +0200
Subject: [PATCH] examples/btmp: use create mode 0660
... to make the created file accessible by the utmp group.
Bug: https://bugzilla.redhat.com/1745330
Suggested-by: Steve Grubb
Closes #257
Upstream-commit: b1bddec3e73bff4282bcd4845f27ab7b375469da
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
examples/btmp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/btmp b/examples/btmp
index 393ead5..0aa1ae1 100644
--- a/examples/btmp
+++ b/examples/btmp
@@ -2,6 +2,6 @@
/var/log/btmp {
missingok
monthly
- create 0600 root utmp
+ create 0660 root utmp
rotate 1
}
--
2.37.3

View File

@ -0,0 +1,556 @@
From 6db706b51af0a6f6ce28bceaefb4157347d2fa18 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Tue, 20 Apr 2021 17:41:10 +0200
Subject: [PATCH 1/6] Log if keyword is not properly separated
The man page states
Values are separated from directives by whitespace and/or an
optional =.
But logrotate does accept no separator, like
rotate7
Log those occurrences with a normal severity, as this usage is not
intended.
Upstream-commit: 2b588b5ec2e5c27bee857c4abeddafa6a9602ebc
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/config.c b/config.c
index 1bca9e4..8049211 100644
--- a/config.c
+++ b/config.c
@@ -1088,6 +1088,11 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
key = isolateWord(&start, &buf, length);
if (key == NULL)
continue;
+ if (!isspace((unsigned char)*start)) {
+ message(MESS_NORMAL, "%s:%d keyword '%s' not properly"
+ " separated, found %#x\n",
+ configFile, lineNum, key, *start);
+ }
if (!strcmp(key, "compress")) {
newlog->flags |= LOG_FLAG_COMPRESS;
} else if (!strcmp(key, "nocompress")) {
--
2.38.1
From 2a22bf99b41e737fcd8c986be5c4fb761ab101c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Tue, 20 Apr 2021 17:41:12 +0200
Subject: [PATCH 2/6] Log error on keyword parse failure
isolateWord() only fails on OOM and EOF.
Upstream-commit: 326179a901b0a8d10e902cae0abab0c68d7abc98
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/config.c b/config.c
index 8049211..fd6e026 100644
--- a/config.c
+++ b/config.c
@@ -1086,8 +1086,11 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
if (isalpha((unsigned char)*start)) {
free(key);
key = isolateWord(&start, &buf, length);
- if (key == NULL)
+ if (key == NULL) {
+ message(MESS_ERROR, "%s:%d failed to parse keyword\n",
+ configFile, lineNum);
continue;
+ }
if (!isspace((unsigned char)*start)) {
message(MESS_NORMAL, "%s:%d keyword '%s' not properly"
" separated, found %#x\n",
--
2.38.1
From d3b2d0d058d41dd7efccadff8506285af791711c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Tue, 20 Apr 2021 17:41:20 +0200
Subject: [PATCH 3/6] Fail on parse error of required option value
Fail on a parse error of a required option value of the directives
include, extension, addextension, rotate, start, minage, maxage,
shredcycles and su.
Failing is better than silently skipping a directive and running with an
undesired configuration.
Upstream-commit: 906ea11981cb1842538c4aaed395885fda693e47
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 49 ++++++++++++++++++++++++++++++-------------------
1 file changed, 30 insertions(+), 19 deletions(-)
diff --git a/config.c b/config.c
index fd6e026..227feec 100644
--- a/config.c
+++ b/config.c
@@ -1154,8 +1154,11 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
mode_t tmp_mode = NO_MODE;
free(key);
key = isolateLine(&start, &buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ message(MESS_ERROR, "%s:%d failed to parse su option value\n",
+ configFile, lineNum);
+ RAISE_ERROR();
+ }
rv = readModeUidGid(configFile, lineNum, key, "su",
&tmp_mode, &newlog->suUid,
@@ -1268,13 +1271,14 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
free(key);
key = isolateValue(configFile, lineNum, "shred cycles",
&start, &buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ RAISE_ERROR();
+ }
newlog->shred_cycles = (int)strtoul(key, &chptr, 0);
if (*chptr || newlog->shred_cycles < 0) {
message(MESS_ERROR, "%s:%d bad shred cycles '%s'\n",
configFile, lineNum, key);
- goto error;
+ RAISE_ERROR();
}
} else if (!strcmp(key, "hourly")) {
set_criterium(&newlog->criterium, ROT_HOURLY, &criterium_set);
@@ -1309,8 +1313,9 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
free(key);
key = isolateValue(configFile, lineNum, "rotate count", &start,
&buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ RAISE_ERROR();
+ }
newlog->rotateCount = (int)strtol(key, &chptr, 0);
if (*chptr || newlog->rotateCount < -1) {
message(MESS_ERROR,
@@ -1322,8 +1327,9 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
free(key);
key = isolateValue(configFile, lineNum, "start count", &start,
&buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ RAISE_ERROR();
+ }
newlog->logStart = (int)strtoul(key, &chptr, 0);
if (*chptr || newlog->logStart < 0) {
message(MESS_ERROR, "%s:%d bad start count '%s'\n",
@@ -1334,8 +1340,9 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
free(key);
key = isolateValue(configFile, lineNum, "minage count", &start,
&buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ RAISE_ERROR();
+ }
newlog->rotateMinAge = (int)strtoul(key, &chptr, 0);
if (*chptr || newlog->rotateMinAge < 0) {
message(MESS_ERROR, "%s:%d bad minimum age '%s'\n",
@@ -1346,8 +1353,9 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
free(key);
key = isolateValue(configFile, lineNum, "maxage count", &start,
&buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ RAISE_ERROR();
+ }
newlog->rotateAge = (int)strtoul(key, &chptr, 0);
if (*chptr || newlog->rotateAge < 0) {
message(MESS_ERROR, "%s:%d bad maximum age '%s'\n",
@@ -1519,8 +1527,9 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
free(key);
key = isolateValue(configFile, lineNum, "include", &start,
&buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ RAISE_ERROR();
+ }
if (key[0] == '~' && key[1] == '/') {
/* replace '~' with content of $HOME cause low-level functions
@@ -1582,8 +1591,9 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
free(key);
key = isolateValue(configFile, lineNum, "extension name", &start,
&buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ RAISE_ERROR();
+ }
freeLogItem (extension);
newlog->extension = key;
key = NULL;
@@ -1593,8 +1603,9 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
free(key);
key = isolateValue(configFile, lineNum, "addextension name", &start,
&buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ RAISE_ERROR();
+ }
freeLogItem (addextension);
newlog->addextension = key;
key = NULL;
--
2.38.1
From 69d2febc6e6e81e34d944b1652144df2e154965d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Mon, 26 Jul 2021 19:35:00 +0200
Subject: [PATCH 4/6] Do not warn on key value pair separated by only an equal
sign
Do not warn if a configuration directive is specified with the key and
value separated by just an equal sign, like:
size=+2048k
The warning is intended for the usage of:
size2048k
Fixes: 2b588b5e ("Log if keyword is not properly separated")
Fixes: #410
Upstream-commit: a98c38bc867ec59e00625b48262bb3334c8f5728
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/config.c b/config.c
index 227feec..6eb94d4 100644
--- a/config.c
+++ b/config.c
@@ -1091,7 +1091,7 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
configFile, lineNum);
continue;
}
- if (!isspace((unsigned char)*start)) {
+ if (!isspace((unsigned char)*start) && *start != '=') {
message(MESS_NORMAL, "%s:%d keyword '%s' not properly"
" separated, found %#x\n",
configFile, lineNum, key, *start);
--
2.38.1
From 3a1f8e746b2753efe7472580b1db4395553b1d34 Mon Sep 17 00:00:00 2001
From: Felix Wilhelm <fwilhelm@google.com>
Date: Thu, 21 Oct 2021 09:47:57 +0000
Subject: [PATCH 5/6] config.c: enforce stricter parsing of config files
Abort parsing of config files that contain invalid lines.
This makes it harder to abuse logrotate for privilege escalation
attacks where an attacker can partially control a privileged file write.
Upstream-commit: 124e4ca6532b0fe823fa2ec145294547b3aaeb4b
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 7 ++++---
test/Makefile.am | 4 +++-
test/test-0102.sh | 16 ++++++++++++++++
test/test-0103.sh | 16 ++++++++++++++++
test/test-config.102.in | 10 ++++++++++
test/test-config.103.in | 12 ++++++++++++
6 files changed, 61 insertions(+), 4 deletions(-)
create mode 100755 test/test-0102.sh
create mode 100755 test/test-0103.sh
create mode 100644 test/test-config.102.in
create mode 100644 test/test-config.103.in
diff --git a/config.c b/config.c
index 6eb94d4..c0fd4ff 100644
--- a/config.c
+++ b/config.c
@@ -1089,12 +1089,13 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
if (key == NULL) {
message(MESS_ERROR, "%s:%d failed to parse keyword\n",
configFile, lineNum);
- continue;
+ RAISE_ERROR();
}
if (!isspace((unsigned char)*start) && *start != '=') {
- message(MESS_NORMAL, "%s:%d keyword '%s' not properly"
+ message(MESS_ERROR, "%s:%d keyword '%s' not properly"
" separated, found %#x\n",
configFile, lineNum, key, *start);
+ RAISE_ERROR();
}
if (!strcmp(key, "compress")) {
newlog->flags |= LOG_FLAG_COMPRESS;
@@ -1973,7 +1974,7 @@ duperror:
message(MESS_ERROR, "%s:%d lines must begin with a keyword "
"or a filename (possibly in double quotes)\n",
configFile, lineNum);
- state = STATE_SKIP_LINE;
+ RAISE_ERROR();
}
break;
case STATE_SKIP_LINE:
diff --git a/test/Makefile.am b/test/Makefile.am
index d6fb7c8..cd357e5 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -89,7 +89,9 @@ TEST_CASES = \
test-0088.sh \
test-0092.sh \
test-0100.sh \
- test-0101.sh
+ test-0101.sh \
+ test-0102.sh \
+ test-0103.sh
EXTRA_DIST = \
compress \
diff --git a/test/test-0102.sh b/test/test-0102.sh
new file mode 100755
index 0000000..d2550a5
--- /dev/null
+++ b/test/test-0102.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+. ./test-common.sh
+
+cleanup 102
+
+# ------------------------------- Test 102 ------------------------------------
+# test invalid config file with binary content
+preptest test.log 102 1
+
+$RLR test-config.102 --force
+
+if [ $? -eq 0 ]; then
+ echo "No error, but there should be one."
+ exit 3
+fi
diff --git a/test/test-0103.sh b/test/test-0103.sh
new file mode 100755
index 0000000..bccd8ed
--- /dev/null
+++ b/test/test-0103.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+. ./test-common.sh
+
+cleanup 103
+
+# ------------------------------- Test 103 ------------------------------------
+# test invalid config file with unknown keywords
+preptest test.log 103 1
+
+$RLR test-config.103 --force
+
+if [ $? -eq 0 ]; then
+ echo "No error, but there should be one."
+ exit 3
+fi
diff --git a/test/test-config.102.in b/test/test-config.102.in
new file mode 100644
index 0000000..cbca4c4
--- /dev/null
+++ b/test/test-config.102.in
@@ -0,0 +1,10 @@
+ELF
+
+&DIR&/test.log {
+ daily
+ size=0
+
+firstaction
+ /bin/sh -c "echo test123"
+ endscript
+}
diff --git a/test/test-config.103.in b/test/test-config.103.in
new file mode 100644
index 0000000..ef4d19c
--- /dev/null
+++ b/test/test-config.103.in
@@ -0,0 +1,12 @@
+random noise
+a b c d
+a::x
+
+&DIR&/test.log {
+ daily
+ size=0
+
+firstaction
+ /bin/sh -c "echo test123"
+ endscript
+}
--
2.38.1
From 2ad71221cd9e485e4d45df4f28b47072491df120 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Mon, 13 Dec 2021 21:47:16 +0100
Subject: [PATCH 6/6] Add more testcases for stricter configuration parsing
Upstream-commit: 9cbc22b91caff6cfbd1378737c62276bd9ffe3e7
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
test/Makefile.am | 4 +++-
test/test-0102.sh | 5 +++++
test/test-0103.sh | 5 +++++
test/test-0104.sh | 19 +++++++++++++++++++
test/test-0105.sh | 25 +++++++++++++++++++++++++
test/test-config.104.in | 8 ++++++++
test/test-config.105.in | 8 ++++++++
7 files changed, 73 insertions(+), 1 deletion(-)
create mode 100755 test/test-0104.sh
create mode 100755 test/test-0105.sh
create mode 100644 test/test-config.104.in
create mode 100644 test/test-config.105.in
diff --git a/test/Makefile.am b/test/Makefile.am
index cd357e5..f1a0062 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -91,7 +91,9 @@ TEST_CASES = \
test-0100.sh \
test-0101.sh \
test-0102.sh \
- test-0103.sh
+ test-0103.sh \
+ test-0104.sh \
+ test-0105.sh
EXTRA_DIST = \
compress \
diff --git a/test/test-0102.sh b/test/test-0102.sh
index d2550a5..367bde9 100755
--- a/test/test-0102.sh
+++ b/test/test-0102.sh
@@ -14,3 +14,8 @@ if [ $? -eq 0 ]; then
echo "No error, but there should be one."
exit 3
fi
+
+checkoutput <<EOF
+test.log 0 zero
+test.log.1 0 first
+EOF
diff --git a/test/test-0103.sh b/test/test-0103.sh
index bccd8ed..32a3c19 100755
--- a/test/test-0103.sh
+++ b/test/test-0103.sh
@@ -14,3 +14,8 @@ if [ $? -eq 0 ]; then
echo "No error, but there should be one."
exit 3
fi
+
+checkoutput <<EOF
+test.log 0 zero
+test.log.1 0 first
+EOF
diff --git a/test/test-0104.sh b/test/test-0104.sh
new file mode 100755
index 0000000..e3c0009
--- /dev/null
+++ b/test/test-0104.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+. ./test-common.sh
+
+cleanup 104
+
+# ------------------------------- Test 104 ------------------------------------
+# test config with unknown (new?) keyword
+preptest test1.log 104 1
+preptest test2.log 104 1
+
+$RLR test-config.104 --force || exit 23
+
+checkoutput <<EOF
+test1.log 0
+test1.log.1 0 zero
+test2.log 0
+test2.log.1 0 zero
+EOF
diff --git a/test/test-0105.sh b/test/test-0105.sh
new file mode 100755
index 0000000..b51e9be
--- /dev/null
+++ b/test/test-0105.sh
@@ -0,0 +1,25 @@
+#!/bin/sh
+
+. ./test-common.sh
+
+cleanup 105
+
+# ------------------------------- Test 105 ------------------------------------
+# test config with garbage keyword bails out
+preptest test1.log 105 1
+preptest test2.log 105 1
+
+$RLR test-config.105 --force
+
+if [ $? -eq 0 ]; then
+ echo "No error, but there should be one."
+ exit 3
+fi
+
+
+checkoutput <<EOF
+test1.log 0 zero
+test1.log.1 0 first
+test2.log 0
+test2.log.1 0 zero
+EOF
diff --git a/test/test-config.104.in b/test/test-config.104.in
new file mode 100644
index 0000000..988d902
--- /dev/null
+++ b/test/test-config.104.in
@@ -0,0 +1,8 @@
+&DIR&/test1.log {
+ newkeyword
+ rotate 1
+}
+
+&DIR&/test2.log {
+ rotate 1
+}
diff --git a/test/test-config.105.in b/test/test-config.105.in
new file mode 100644
index 0000000..bfab9b9
--- /dev/null
+++ b/test/test-config.105.in
@@ -0,0 +1,8 @@
+&DIR&/test1.log {
+ g@rbag€[]+#*
+ rotate 1
+}
+
+&DIR&/test2.log {
+ rotate 1
+}
--
2.38.1

View File

@ -1,635 +0,0 @@
From 92067ac8e75b3d1f431982d8156da5ffb18df249 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Thu, 7 Jun 2018 14:49:07 +0200
Subject: [PATCH 1/7] return non-zero exit status if a config file contains an
error
... which causes the config file to be skipped
Closes #199
Closes #204
Upstream-commit: e547b942ebdf58026f0b28a74b3d02a7674e38dc
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 1 +
test/Makefile.am | 1 +
test/test-0083.sh | 14 ++++++++++++++
test/test-config.83.in | 3 +++
4 files changed, 19 insertions(+)
create mode 100755 test/test-0083.sh
create mode 100644 test/test-config.83.in
diff --git a/config.c b/config.c
index 1805a16..ec4c5fb 100644
--- a/config.c
+++ b/config.c
@@ -1820,6 +1820,7 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
message(MESS_ERROR, "found error in %s, skipping\n",
newlog->pattern ? newlog->pattern : "log config");
+ logerror = 1;
state = STATE_SKIP_CONFIG;
break;
case STATE_LOAD_SCRIPT:
diff --git a/test/Makefile.am b/test/Makefile.am
index 35ba2b9..cfe09c4 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -76,6 +76,7 @@ TEST_CASES = \
test-0075.sh \
test-0076.sh \
test-0077.sh \
+ test-0083.sh \
test-0084.sh \
test-0100.sh \
test-0101.sh
diff --git a/test/test-0083.sh b/test/test-0083.sh
new file mode 100755
index 0000000..f6cf26c
--- /dev/null
+++ b/test/test-0083.sh
@@ -0,0 +1,14 @@
+#!/bin/bash
+
+. ./test-common.sh
+
+cleanup 83
+
+# ------------------------------- Test 83 ------------------------------------
+preptest test.log 83 1
+
+if $RLR test-config.83 -v --force; then
+ exit 1
+else
+ exit 0
+fi
diff --git a/test/test-config.83.in b/test/test-config.83.in
new file mode 100644
index 0000000..f8a36f8
--- /dev/null
+++ b/test/test-config.83.in
@@ -0,0 +1,3 @@
+&DIR&/test.log {
+ rotate 1 # invalid comment
+}
--
2.38.1
From d6b10a7dd5946a6bce400ab87fd1adbde832c046 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Tue, 20 Apr 2021 17:41:10 +0200
Subject: [PATCH 2/7] Log if keyword is not properly separated
The man page states
Values are separated from directives by whitespace and/or an
optional =.
But logrotate does accept no separator, like
rotate7
Log those occurrences with a normal severity, as this usage is not
intended.
Upstream-commit: 2b588b5ec2e5c27bee857c4abeddafa6a9602ebc
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/config.c b/config.c
index ec4c5fb..cfbb3d1 100644
--- a/config.c
+++ b/config.c
@@ -1047,6 +1047,11 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
key = isolateWord(&start, &buf, length);
if (key == NULL)
continue;
+ if (!isspace((unsigned char)*start)) {
+ message(MESS_NORMAL, "%s:%d keyword '%s' not properly"
+ " separated, found %#x\n",
+ configFile, lineNum, key, *start);
+ }
if (!strcmp(key, "compress")) {
newlog->flags |= LOG_FLAG_COMPRESS;
} else if (!strcmp(key, "nocompress")) {
--
2.38.1
From 0881276c62ac95d803371f3f5c6cf11ffb552211 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Tue, 20 Apr 2021 17:41:12 +0200
Subject: [PATCH 3/7] Log error on keyword parse failure
isolateWord() only fails on OOM and EOF.
Upstream-commit: 326179a901b0a8d10e902cae0abab0c68d7abc98
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/config.c b/config.c
index cfbb3d1..5a774ac 100644
--- a/config.c
+++ b/config.c
@@ -1045,8 +1045,11 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
if (isalpha((unsigned char)*start)) {
free(key);
key = isolateWord(&start, &buf, length);
- if (key == NULL)
+ if (key == NULL) {
+ message(MESS_ERROR, "%s:%d failed to parse keyword\n",
+ configFile, lineNum);
continue;
+ }
if (!isspace((unsigned char)*start)) {
message(MESS_NORMAL, "%s:%d keyword '%s' not properly"
" separated, found %#x\n",
--
2.38.1
From 539b863fbd211b61614493447040cb340b53f0c0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Tue, 20 Apr 2021 17:41:20 +0200
Subject: [PATCH 4/7] Fail on parse error of required option value
Fail on a parse error of a required option value of the directives
include, extension, addextension, rotate, start, minage, maxage,
shredcycles and su.
Failing is better than silently skipping a directive and running with an
undesired configuration.
Upstream-commit: 906ea11981cb1842538c4aaed395885fda693e47
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 49 ++++++++++++++++++++++++++++++-------------------
1 file changed, 30 insertions(+), 19 deletions(-)
diff --git a/config.c b/config.c
index 5a774ac..ae7bf4b 100644
--- a/config.c
+++ b/config.c
@@ -1110,8 +1110,11 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
mode_t tmp_mode = NO_MODE;
free(key);
key = isolateLine(&start, &buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ message(MESS_ERROR, "%s:%d failed to parse su option value\n",
+ configFile, lineNum);
+ RAISE_ERROR();
+ }
rv = readModeUidGid(configFile, lineNum, key, "su",
&tmp_mode, &newlog->suUid,
@@ -1209,13 +1212,14 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
free(key);
key = isolateValue(configFile, lineNum, "shred cycles",
&start, &buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ RAISE_ERROR();
+ }
newlog->shred_cycles = strtoul(key, &chptr, 0);
if (*chptr || newlog->shred_cycles < 0) {
message(MESS_ERROR, "%s:%d bad shred cycles '%s'\n",
configFile, lineNum, key);
- goto error;
+ RAISE_ERROR();
}
} else if (!strcmp(key, "hourly")) {
newlog->criterium = ROT_HOURLY;
@@ -1250,8 +1254,9 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
free(key);
key = isolateValue(configFile, lineNum, "rotate count", &start,
&buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ RAISE_ERROR();
+ }
newlog->rotateCount = strtoul(key, &chptr, 0);
if (*chptr || newlog->rotateCount < 0) {
message(MESS_ERROR,
@@ -1263,8 +1268,9 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
free(key);
key = isolateValue(configFile, lineNum, "start count", &start,
&buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ RAISE_ERROR();
+ }
newlog->logStart = strtoul(key, &chptr, 0);
if (*chptr || newlog->logStart < 0) {
message(MESS_ERROR, "%s:%d bad start count '%s'\n",
@@ -1275,8 +1281,9 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
free(key);
key = isolateValue(configFile, lineNum, "minage count", &start,
&buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ RAISE_ERROR();
+ }
newlog->rotateMinAge = strtoul(key, &chptr, 0);
if (*chptr || newlog->rotateMinAge < 0) {
message(MESS_ERROR, "%s:%d bad minimum age '%s'\n",
@@ -1287,8 +1294,9 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
free(key);
key = isolateValue(configFile, lineNum, "maxage count", &start,
&buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ RAISE_ERROR();
+ }
newlog->rotateAge = strtoul(key, &chptr, 0);
if (*chptr || newlog->rotateAge < 0) {
message(MESS_ERROR, "%s:%d bad maximum age '%s'\n",
@@ -1443,8 +1451,9 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
free(key);
key = isolateValue(configFile, lineNum, "include", &start,
&buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ RAISE_ERROR();
+ }
message(MESS_DEBUG, "including %s\n", key);
if (recursion_depth >= MAX_NESTING) {
message(MESS_ERROR, "%s:%d include nesting too deep\n",
@@ -1473,8 +1482,9 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
free(key);
key = isolateValue(configFile, lineNum, "extension name", &start,
&buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ RAISE_ERROR();
+ }
freeLogItem (extension);
newlog->extension = key;
key = NULL;
@@ -1484,8 +1494,9 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
free(key);
key = isolateValue(configFile, lineNum, "addextension name", &start,
&buf, length);
- if (key == NULL)
- continue;
+ if (key == NULL) {
+ RAISE_ERROR();
+ }
freeLogItem (addextension);
newlog->addextension = key;
key = NULL;
--
2.38.1
From bf20b227b45b232eec9b659839d7ae20604f5de3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Mon, 26 Jul 2021 19:35:00 +0200
Subject: [PATCH 5/7] Do not warn on key value pair separated by only an equal
sign
Do not warn if a configuration directive is specified with the key and
value separated by just an equal sign, like:
size=+2048k
The warning is intended for the usage of:
size2048k
Fixes: 2b588b5e ("Log if keyword is not properly separated")
Fixes: #410
Upstream-commit: a98c38bc867ec59e00625b48262bb3334c8f5728
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/config.c b/config.c
index ae7bf4b..569104d 100644
--- a/config.c
+++ b/config.c
@@ -1050,7 +1050,7 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
configFile, lineNum);
continue;
}
- if (!isspace((unsigned char)*start)) {
+ if (!isspace((unsigned char)*start) && *start != '=') {
message(MESS_NORMAL, "%s:%d keyword '%s' not properly"
" separated, found %#x\n",
configFile, lineNum, key, *start);
--
2.38.1
From 07faa84dc2e31002b0212c0b57669595ef9be99d Mon Sep 17 00:00:00 2001
From: Felix Wilhelm <fwilhelm@google.com>
Date: Thu, 21 Oct 2021 09:47:57 +0000
Subject: [PATCH 6/7] config.c: enforce stricter parsing of config files
Abort parsing of config files that contain invalid lines.
This makes it harder to abuse logrotate for privilege escalation
attacks where an attacker can partially control a privileged file write.
Upstream-commit: 124e4ca6532b0fe823fa2ec145294547b3aaeb4b
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
config.c | 7 ++++---
test/Makefile.am | 4 +++-
test/test-0102.sh | 16 ++++++++++++++++
test/test-0103.sh | 16 ++++++++++++++++
test/test-config.102.in | 10 ++++++++++
test/test-config.103.in | 12 ++++++++++++
6 files changed, 61 insertions(+), 4 deletions(-)
create mode 100755 test/test-0102.sh
create mode 100755 test/test-0103.sh
create mode 100644 test/test-config.102.in
create mode 100644 test/test-config.103.in
diff --git a/config.c b/config.c
index 569104d..36765be 100644
--- a/config.c
+++ b/config.c
@@ -1048,12 +1048,13 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
if (key == NULL) {
message(MESS_ERROR, "%s:%d failed to parse keyword\n",
configFile, lineNum);
- continue;
+ RAISE_ERROR();
}
if (!isspace((unsigned char)*start) && *start != '=') {
- message(MESS_NORMAL, "%s:%d keyword '%s' not properly"
+ message(MESS_ERROR, "%s:%d keyword '%s' not properly"
" separated, found %#x\n",
configFile, lineNum, key, *start);
+ RAISE_ERROR();
}
if (!strcmp(key, "compress")) {
newlog->flags |= LOG_FLAG_COMPRESS;
@@ -1805,7 +1806,7 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
message(MESS_ERROR, "%s:%d lines must begin with a keyword "
"or a filename (possibly in double quotes)\n",
configFile, lineNum);
- state = STATE_SKIP_LINE;
+ RAISE_ERROR();
}
break;
case STATE_SKIP_LINE:
diff --git a/test/Makefile.am b/test/Makefile.am
index cfe09c4..255c1f7 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -79,7 +79,9 @@ TEST_CASES = \
test-0083.sh \
test-0084.sh \
test-0100.sh \
- test-0101.sh
+ test-0101.sh \
+ test-0102.sh \
+ test-0103.sh
EXTRA_DIST = \
compress \
diff --git a/test/test-0102.sh b/test/test-0102.sh
new file mode 100755
index 0000000..d2550a5
--- /dev/null
+++ b/test/test-0102.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+. ./test-common.sh
+
+cleanup 102
+
+# ------------------------------- Test 102 ------------------------------------
+# test invalid config file with binary content
+preptest test.log 102 1
+
+$RLR test-config.102 --force
+
+if [ $? -eq 0 ]; then
+ echo "No error, but there should be one."
+ exit 3
+fi
diff --git a/test/test-0103.sh b/test/test-0103.sh
new file mode 100755
index 0000000..bccd8ed
--- /dev/null
+++ b/test/test-0103.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+. ./test-common.sh
+
+cleanup 103
+
+# ------------------------------- Test 103 ------------------------------------
+# test invalid config file with unknown keywords
+preptest test.log 103 1
+
+$RLR test-config.103 --force
+
+if [ $? -eq 0 ]; then
+ echo "No error, but there should be one."
+ exit 3
+fi
diff --git a/test/test-config.102.in b/test/test-config.102.in
new file mode 100644
index 0000000..cbca4c4
--- /dev/null
+++ b/test/test-config.102.in
@@ -0,0 +1,10 @@
+ELF
+
+&DIR&/test.log {
+ daily
+ size=0
+
+firstaction
+ /bin/sh -c "echo test123"
+ endscript
+}
diff --git a/test/test-config.103.in b/test/test-config.103.in
new file mode 100644
index 0000000..ef4d19c
--- /dev/null
+++ b/test/test-config.103.in
@@ -0,0 +1,12 @@
+random noise
+a b c d
+a::x
+
+&DIR&/test.log {
+ daily
+ size=0
+
+firstaction
+ /bin/sh -c "echo test123"
+ endscript
+}
--
2.38.1
From 88870bf2d84f65d0f2633bb32b7dc696be51d202 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Mon, 13 Dec 2021 21:47:16 +0100
Subject: [PATCH 7/7] Add more testcases for stricter configuration parsing
Upstream-commit: 9cbc22b91caff6cfbd1378737c62276bd9ffe3e7
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
test/Makefile.am | 4 +++-
test/test-0102.sh | 5 +++++
test/test-0103.sh | 5 +++++
test/test-0104.sh | 19 +++++++++++++++++++
test/test-0105.sh | 25 +++++++++++++++++++++++++
test/test-config.104.in | 8 ++++++++
test/test-config.105.in | 8 ++++++++
7 files changed, 73 insertions(+), 1 deletion(-)
create mode 100755 test/test-0104.sh
create mode 100755 test/test-0105.sh
create mode 100644 test/test-config.104.in
create mode 100644 test/test-config.105.in
diff --git a/test/Makefile.am b/test/Makefile.am
index 255c1f7..a489a76 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -81,7 +81,9 @@ TEST_CASES = \
test-0100.sh \
test-0101.sh \
test-0102.sh \
- test-0103.sh
+ test-0103.sh \
+ test-0104.sh \
+ test-0105.sh
EXTRA_DIST = \
compress \
diff --git a/test/test-0102.sh b/test/test-0102.sh
index d2550a5..367bde9 100755
--- a/test/test-0102.sh
+++ b/test/test-0102.sh
@@ -14,3 +14,8 @@ if [ $? -eq 0 ]; then
echo "No error, but there should be one."
exit 3
fi
+
+checkoutput <<EOF
+test.log 0 zero
+test.log.1 0 first
+EOF
diff --git a/test/test-0103.sh b/test/test-0103.sh
index bccd8ed..32a3c19 100755
--- a/test/test-0103.sh
+++ b/test/test-0103.sh
@@ -14,3 +14,8 @@ if [ $? -eq 0 ]; then
echo "No error, but there should be one."
exit 3
fi
+
+checkoutput <<EOF
+test.log 0 zero
+test.log.1 0 first
+EOF
diff --git a/test/test-0104.sh b/test/test-0104.sh
new file mode 100755
index 0000000..e3c0009
--- /dev/null
+++ b/test/test-0104.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+. ./test-common.sh
+
+cleanup 104
+
+# ------------------------------- Test 104 ------------------------------------
+# test config with unknown (new?) keyword
+preptest test1.log 104 1
+preptest test2.log 104 1
+
+$RLR test-config.104 --force || exit 23
+
+checkoutput <<EOF
+test1.log 0
+test1.log.1 0 zero
+test2.log 0
+test2.log.1 0 zero
+EOF
diff --git a/test/test-0105.sh b/test/test-0105.sh
new file mode 100755
index 0000000..b51e9be
--- /dev/null
+++ b/test/test-0105.sh
@@ -0,0 +1,25 @@
+#!/bin/sh
+
+. ./test-common.sh
+
+cleanup 105
+
+# ------------------------------- Test 105 ------------------------------------
+# test config with garbage keyword bails out
+preptest test1.log 105 1
+preptest test2.log 105 1
+
+$RLR test-config.105 --force
+
+if [ $? -eq 0 ]; then
+ echo "No error, but there should be one."
+ exit 3
+fi
+
+
+checkoutput <<EOF
+test1.log 0 zero
+test1.log.1 0 first
+test2.log 0
+test2.log.1 0 zero
+EOF
diff --git a/test/test-config.104.in b/test/test-config.104.in
new file mode 100644
index 0000000..988d902
--- /dev/null
+++ b/test/test-config.104.in
@@ -0,0 +1,8 @@
+&DIR&/test1.log {
+ newkeyword
+ rotate 1
+}
+
+&DIR&/test2.log {
+ rotate 1
+}
diff --git a/test/test-config.105.in b/test/test-config.105.in
new file mode 100644
index 0000000..bfab9b9
--- /dev/null
+++ b/test/test-config.105.in
@@ -0,0 +1,8 @@
+&DIR&/test1.log {
+ g@rbag€[]+#*
+ rotate 1
+}
+
+&DIR&/test2.log {
+ rotate 1
+}
--
2.38.1

View File

@ -1,46 +1,46 @@
Summary: Rotates, compresses, removes and mails system log files
Name: logrotate
Version: 3.14.0
Release: 6%{?dist}
Version: 3.18.0
Release: 8%{?dist}
License: GPLv2+
Url: https://github.com/logrotate/logrotate
Source: https://github.com/logrotate/logrotate/releases/download/%{version}/logrotate-%{version}.tar.xz
URL: https://github.com/logrotate/logrotate
Source0: https://github.com/logrotate/logrotate/releases/download/%{version}/logrotate-%{version}.tar.xz
Source1: rwtab
# fix resource leaks reported by Coverity
Patch: 0001-logrotate-3.18.0-fix-resource-leaks.patch
# unify documentation of copy/copytruncate/renamecopy (#1934629)
Patch: 0002-logrotate-3.18.0-copytruncate-doc.patch
# make `renamecopy` and `copytruncate` override each other (#1934601)
Patch: 0003-logrotate-3.18.0-renamecopy-excl.patch
# fix potential DoS from unprivileged users via the state file (CVE-2022-1348)
Patch: 0004-logrotate-3.18.0-CVE-2022-1348.patch
# enforce stricter parsing of config files (#2148925)
Patch: 0005-logrotate-3.18.0-stricter-config-parser.patch
BuildRequires: acl
BuildRequires: automake
BuildRequires: gcc
BuildRequires: git
BuildRequires: libacl-devel
BuildRequires: libselinux-devel
BuildRequires: make
BuildRequires: popt-devel
BuildRequires: systemd-rpm-macros
Requires: coreutils
# document the --version option in the logrotate(8) man page (#1611498)
Patch1: 0001-logrotate-3.14.0-man-version.patch
# fix programming mistakes detected by Coverity Analysis
Patch2: 0002-logrotate-3.14.0-coverity.patch
# do not abort globbing on broken symlink (#1723265)
Patch3: 0003-logrotate-3.14.0-broken-symlink.patch
# logrotate.8: encourage admins to use the `su` directive (#1759770)
Patch4: 0004-logrotate-3.14.0-man-page-su.patch
# create /var/log/btmp with mode 0660 (#2061561)
Patch5: 0005-logrotate-3.14.0-btmp-create-mode.patch
# enforce stricter parsing of config files (#2148925)
Patch6: 0006-logrotate-3.14.0-stricter-config-parser.patch
Requires(post): systemd
Requires(preun): systemd
%description
The logrotate utility is designed to simplify the administration of
log files on a system which generates a lot of log files. Logrotate
allows for the automatic rotation compression, removal and mailing of
log files. Logrotate can be set to handle a log file daily, weekly,
monthly or when the log file gets to a certain size. Normally,
logrotate runs as a daily cron job.
monthly or when the log file gets to a certain size.
Install the logrotate package if you need a utility to deal with the
log files on your system.
@ -56,14 +56,6 @@ EOF
git add .gitignore
git commit -m "update .gitignore"
%if 0%{?fedora} == 0 && 0%{?rhel} < 7
sed -e 's/^AM_EXTRA_RECURSIVE_TARGETS/dnl AM_EXTRA_RECURSIVE_TARGETS/' \
-e 's/ serial-tests//' \
-i configure.ac
git add configure.ac
git commit -m "configure.ac: compatibility fixes for RHEL-6"
%endif
autoreconf -fiv
git add --all
git commit -m "force autoreconf" --allow-empty
@ -72,21 +64,21 @@ git commit -m "force autoreconf" --allow-empty
mkdir build && cd build
%global _configure ../configure
%configure --with-state-file-path=%{_localstatedir}/lib/logrotate/logrotate.status
make %{?_smp_mflags} V=1
%make_build
%check
make %{?_smp_mflags} -C build check
%make_build -C build -s check
%install
%make_install -C build
mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/logrotate.d
mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/cron.daily
mkdir -p $RPM_BUILD_ROOT%{_unitdir}
mkdir -p $RPM_BUILD_ROOT%{_localstatedir}/lib/logrotate
install -p -m 644 examples/logrotate-default $RPM_BUILD_ROOT%{_sysconfdir}/logrotate.conf
install -p -m 644 examples/logrotate.conf $RPM_BUILD_ROOT%{_sysconfdir}/
install -p -m 644 examples/{b,w}tmp $RPM_BUILD_ROOT%{_sysconfdir}/logrotate.d/
install -p -m 755 examples/logrotate.cron $RPM_BUILD_ROOT%{_sysconfdir}/cron.daily/logrotate
install -p -m 644 examples/logrotate.{service,timer} $RPM_BUILD_ROOT%{_unitdir}/
# Make sure logrotate is able to run on read-only root
mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/rwtab.d
@ -102,37 +94,103 @@ if [ ! -d %{_localstatedir}/lib/logrotate/ -a -f %{_localstatedir}/lib/logrotate
cp -a %{_localstatedir}/lib/logrotate.status %{_localstatedir}/lib/logrotate
fi
%post
%systemd_post logrotate.{service,timer}
# If there is any cron daemon configured, enable the systemd timer to avoid
# breaking the configuration silently when upgrading from 3.14.0-4 or
# earlier versions
%triggerin -- logrotate < 3.14.0-5
[ -e %{_sysconfdir}/crontab -o -e %{_sysconfdir}/anacrontab -o -e %{_sysconfdir}/fcrontab ] \
&& %{_bindir}/systemctl enable --now logrotate.timer &>/dev/null || :
%preun
%systemd_preun logrotate.{service,timer}
%files
%{!?_licensedir:%global license %%doc}
%license COPYING
%doc ChangeLog.md
%{_sbindir}/logrotate
%{_unitdir}/logrotate.{service,timer}
%{_mandir}/man8/logrotate.8*
%{_mandir}/man5/logrotate.conf.5*
%dir %{_sysconfdir}/cron.daily
%config(noreplace) %{_sysconfdir}/cron.daily/logrotate
%config(noreplace) %{_sysconfdir}/logrotate.conf
%dir %{_sysconfdir}/logrotate.d
%config(noreplace) %{_sysconfdir}/logrotate.d/{b,w}tmp
%dir %{_localstatedir}/lib/logrotate
%ghost %verify(not size md5 mtime) %attr(0644, root, root) %{_localstatedir}/lib/logrotate/logrotate.status
%ghost %verify(not size md5 mtime) %attr(0640, root, root) %{_localstatedir}/lib/logrotate/logrotate.status
%config(noreplace) %{_sysconfdir}/rwtab.d/logrotate
%changelog
* Tue Dec 20 2022 Kamil Dudka <kdudka@redhat.com> - 3.14.0-6
* Tue Dec 20 2022 Kamil Dudka <kdudka@redhat.com> - 3.18.0-8
- enforce stricter parsing of config files (#2148925)
* Mon Nov 14 2022 Kamil Dudka <kdudka@redhat.com> - 3.14.0-5
- create /var/log/btmp with mode 0660 (#2061561)
* Fri May 27 2022 Kamil Dudka <kdudka@redhat.com> - 3.18.0-7
- lockState: do not print `error:` when exit code is unaffected (#2090926)
* Wed May 06 2020 Kamil Dudka <kdudka@redhat.com> - 3.14.0-4
- logrotate.8: encourage admins to use the `su` directive (#1759770)
- do not abort globbing on broken symlink (#1723265)
* Wed May 25 2022 Kamil Dudka <kdudka@redhat.com> - 3.18.0-6
- fix potential DoS from unprivileged users via the state file (CVE-2022-1348)
* Fri Aug 10 2018 Kamil Dudka <kdudka@redhat.com> - 3.14.0-3
* Mon Aug 09 2021 Mohan Boddu <mboddu@redhat.com>
- Rebuilt for IMA sigs, glibc 2.34, aarch64 flags
Related: rhbz#1991688
* Tue May 04 2021 Kamil Dudka <kdudka@redhat.com> - 3.18.0-4
- make `renamecopy` and `copytruncate` override each other (#1934601)
- unify documentation of copy/copytruncate/renamecopy (#1934629)
- fix resource leaks reported by Coverity
* Fri Apr 16 2021 Mohan Boddu <mboddu@redhat.com> - 3.18.0-3
- Rebuilt for RHEL 9 BETA on Apr 15th 2021. Related: rhbz#1947937
* Tue Jan 26 2021 Fedora Release Engineering <releng@fedoraproject.org> - 3.18.0-2
- Rebuilt for https://fedoraproject.org/wiki/Fedora_34_Mass_Rebuild
* Fri Jan 08 2021 Kamil Dudka <kdudka@redhat.com> - 3.18.0-1
- new upstream version 3.18.0
* Tue Jul 28 2020 Fedora Release Engineering <releng@fedoraproject.org> - 3.17.0-3
- Rebuilt for https://fedoraproject.org/wiki/Fedora_33_Mass_Rebuild
* Mon Jul 13 2020 Tom Stellard <tstellar@redhat.com> - 3.17.0-2
- Use make macros
- https://fedoraproject.org/wiki/Changes/UseMakeBuildInstallMacro
* Fri Jul 10 2020 Kamil Dudka <kdudka@redhat.com> - 3.17.0-1
- new upstream version 3.17.0
* Fri Feb 28 2020 Kamil Dudka <kdudka@redhat.com> - 3.16.0-1
- new upstream version 3.16.0
* Thu Jan 30 2020 Kamil Dudka <kdudka@redhat.com> - 3.15.1-3
- make the code compile with gcc-10
* Wed Jan 29 2020 Fedora Release Engineering <releng@fedoraproject.org> - 3.15.1-2
- Rebuilt for https://fedoraproject.org/wiki/Fedora_32_Mass_Rebuild
* Fri Aug 30 2019 Kamil Dudka <kdudka@redhat.com> - 3.15.1-1
- new upstream version 3.15.1
* Thu Jul 25 2019 Fedora Release Engineering <releng@fedoraproject.org> - 3.15.0-3
- Rebuilt for https://fedoraproject.org/wiki/Fedora_31_Mass_Rebuild
* Fri Feb 01 2019 Fedora Release Engineering <releng@fedoraproject.org> - 3.15.0-2
- Rebuilt for https://fedoraproject.org/wiki/Fedora_30_Mass_Rebuild
* Tue Dec 04 2018 Kamil Dudka <kdudka@redhat.com> - 3.15.0-1
- new upstream version 3.15.0
* Wed Nov 21 2018 Alejandro Domínguez Muñoz <adomu@net-c.com> - 3.14.0-5
- add make as a build dependency
- replace cron job with a systemd timer unit (#1502085, #1655153)
* Fri Aug 10 2018 Kamil Dudka <kdudka@redhat.com> - 3.14.0-4
- fix programming mistakes detected by Coverity Analysis
- document the --version option in the logrotate(8) man page (#1611498)
* Fri Jul 13 2018 Fedora Release Engineering <releng@fedoraproject.org> - 3.14.0-3
- Rebuilt for https://fedoraproject.org/wiki/Fedora_29_Mass_Rebuild
* Wed Jul 11 2018 Kamil Dudka <kdudka@redhat.com> - 3.14.0-2
- fix license tag to match the source code license