File sarg-2.4.0-avoid-race-condition-when-creating-t... of Package sarg

Overview Repositories Revisions Requests Users Attributes Meta

File sarg-2.4.0-avoid-race-condition-when-creating-the-temporary-dir.patch of Package sarg (Revision 8b7bbacf9824f7abf0c0319b0e7a2430)

Currently displaying revision 8b7bbacf9824f7abf0c0319b0e7a2430 , Show latest

From 8ec6d20be8c0da3c885aba78e63251f2e5080748 Mon Sep 17 00:00:00 2001
From: Frederic Marchal <fmarchal@users.sourceforge.net>
Date: Tue, 24 Dec 2019 12:03:06 +0100
Subject: [PATCH] Avoid race condition when creating the temporary directory

If the temporary directory exists, its content must be checked and purged
after creating the temporary directory. Doing the reverse (as was the case
before) opens the door to a race condition where a malicious user replaces
the temporary directory just after its content was checked and deemed to be
safe.

Thanks to Matthias Gerstner for reporting this issue.
---
 include/defs.h |  3 ++-
 log.c          |  6 +-----
 util.c         | 27 ++++++++++++++++++++++++++-
 3 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/include/defs.h b/include/defs.h
index 9e0bf5a..9010615 100644
--- a/include/defs.h
+++ b/include/defs.h
@@ -419,7 +419,8 @@ void version(void);
 int vercode(const char *code);
 void load_excludecodes(const char *ExcludeCodes);
 void free_excludecodes(void);
-void my_mkdir(const char *name);
+bool my_mkdir(const char *name);
+void makeTmpDir(const char *tmp);
 int testvaliduserchar(const char *user);
 char *strlow(char *string);
 char *strup(char *string);
diff --git a/log.c b/log.c
index df397e8..4a6fa51 100644
--- a/log.c
+++ b/log.c
@@ -583,11 +583,7 @@ int main(int argc,char *argv[])
 		strcat(outdir,"/");
 	}
 
-	if(access(tmp, R_OK) == 0) {
-		if (debug) debuga(_("Deleting temporary directory \"%s\"\n"),tmp);
-		emptytmpdir(tmp);
-	}
-	my_mkdir(tmp);
+	makeTmpDir(tmp);
 	snprintf(denied_unsort,sizeof(denied_unsort),"%s/denied.int_unsort",tmp);
 	snprintf(denied_sort,sizeof(denied_sort),"%s/denied.int_log",tmp);
 	snprintf(authfail_unsort,sizeof(authfail_unsort),"%s/authfail.int_unsort",tmp);
diff --git a/util.c b/util.c
index bc4a15d..d50f6f4 100644
--- a/util.c
+++ b/util.c
@@ -411,11 +411,17 @@ int PortableMkDir(const char *path,int mode)
 	return(0);
 }
 
-void my_mkdir(const char *name)
+/*!
+ * Recursively create a path by adding missing directory until the whole path is created.
+ * \param name The path to create.
+ * \return True if the directory was created or false if it already existed
+ */
+bool my_mkdir(const char *name)
 {
 	char w0[MAXLEN];
 	int i;
 	int chars;
+	bool created = false;
 
 	if(!is_absolute(name)) {
 		debuga(_("Invalid path \"%s\". Please, use absolute paths only.\n"),name);
@@ -447,9 +453,28 @@ void my_mkdir(const char *name)
 			debuga(_("Cannot create directory \"%s\": %s\n"),name,strerror(errno));
 			exit(EXIT_FAILURE);
 		}
+		created = true;
 	}
+	return created;
 }
 
+void makeTmpDir(const char *tmp)
+{
+	/*
+	 * We must ensure the temporary directory is ours. In particular, we must make sure no malicious
+	 * users managed to create or replace the temporary directory with a symlink to a system directory.
+	 * As sarg purges the content of the temporary directory upon exit, should the temporary directory
+	 * be hijacked, sarg could be tricked in deleting system files such as /bin or users files in /home
+	 * or logs in /var/log.
+	 *
+	 * The code first create the temporary directory. If it wasn't created, the content is checked and
+	 * purged if it looks safe to delete every file and directory it contains.
+	 */
+	if (!my_mkdir(tmp)) {
+		if (debug) debuga(__FILE__, __LINE__, _("Deleting temporary directory \"%s\"\n"), tmp);
+		emptytmpdir(tmp);
+	}
+}
 
 void my_lltoa(unsigned long long int n, char *s, int ssize, int len)
 {
-- 
2.16.4

Places

File sarg-2.4.0-avoid-race-condition-when-creating-the-temporary-dir.patch of Package sarg (Revision 8b7bbacf9824f7abf0c0319b0e7a2430)

Places