File sarg-2.4.0-use-unpredictable-temporary-directory-name.patch of Package sarg
From 332f0c12aad48cf36d92960eb5b21fdbc6842ba8 Mon Sep 17 00:00:00 2001
From: Frederic Marchal <fmarchal@users.sourceforge.net>
Date: Tue, 24 Dec 2019 13:49:19 +0100
Subject: [PATCH] Use unpredictable temporary directory name
It might be unsafe to use a predictable temporary directory (default to
/tmp/sarg) under the world writable /tmp directory.
To mitigate attacks exploiting the knowledge of the temporary directory,
a random temporary directory is safely created by the system.
Fixed known temporary directory can still be used with temporary_dir_path
parameter in sarg.conf.
---
getconf.c | 2 ++
include/conf.h | 1 +
include/defs.h | 1 +
log.c | 34 ++++++++++++++++++++++++++--------
sarg.conf | 17 +++++++++++++++++
util.c | 26 +++++++++++++++++++++++++-
6 files changed, 72 insertions(+), 9 deletions(-)
diff --git a/getconf.c b/getconf.c
index 6da4971..ad03f84 100644
--- a/getconf.c
+++ b/getconf.c
@@ -714,6 +714,8 @@ static void parmtest(char *buf,const char *File)
if (getparam_string("temporary_dir",buf,TempDir,sizeof(TempDir))>0) return;
+ if (getparam_string("temporary_dir_path",buf,TempDirPath,sizeof(TempDirPath))>0) return;
+
if (getparam_list("report_type",SET_LIST(report_type_values),buf,&ReportType)>0) return;
if (getparam_string("output_dir",buf,OutputDir,sizeof(OutputDir))>0) return;
diff --git a/include/conf.h b/include/conf.h
index 133ac89..b5e3c6c 100644
--- a/include/conf.h
+++ b/include/conf.h
@@ -353,6 +353,7 @@ char HeaderBgColor[MAXLEN];
char FontSize[MAXLEN];
char PasswdFile[MAXLEN];
char TempDir[MAXLEN];
+char TempDirPath[MAXLEN];
char OutputDir[MAXLEN];
char OutputEmail[MAXLEN];
unsigned long int TopuserSort;
diff --git a/include/defs.h b/include/defs.h
index 9010615..cf2434a 100644
--- a/include/defs.h
+++ b/include/defs.h
@@ -445,3 +445,4 @@ int compar( const void *, const void * );
void unlinkdir(const char *dir,bool contentonly);
void emptytmpdir(const char *dir);
int extract_address_mask(const char *buf,const char **text,unsigned char *ipv4,unsigned short int *ipv6,int *nbits,const char **next);
+void append_to_path(char *base_path, int base_path_size, const char *append);
diff --git a/log.c b/log.c
index 4a6fa51..d009478 100644
--- a/log.c
+++ b/log.c
@@ -49,6 +49,7 @@ bool UserAgentFromCmdLine=false;
char *CurrentLocale=NULL;
static void getusers(const char *pwdfile, int debug);
+static void CleanTemporaryDir();
int main(int argc,char *argv[])
{
@@ -149,6 +150,7 @@ int main(int argc,char *argv[])
strcpy(LogoTextColor,"#006699");
strcpy(FontSize,"9px");
strcpy(TempDir,"/tmp");
+ TempDirPath[0] = '\0';
strcpy(OutputDir,"/var/www/html/squid-reports");
AnonymousOutputFiles=false;
Ip2Name=false;
@@ -565,15 +567,28 @@ int main(int argc,char *argv[])
if(tmp[0] == '\0') strcpy(tmp,TempDir);
else strcpy(TempDir,tmp);
/*
- For historical reasons, the temporary directory is the subdirectory "sarg" of the path
- provided by the user.
+ For historical reasons, the temporary directory used to be subdirectory "sarg" of the path
+ provided by the user. The full temporary directory was the predictable name /tmp/sarg. It is unsafe
+ to use a predictable name in the world writable /tmp as malicious users might use that knowledge
+ to lure sarg into some kind of nasty activity it was not designed for.
+ The default is now to use a random name safely created by the system but it is still possible to
+ use a known fixed path set with a parameter in sarg.conf.
*/
- strcat(tmp,"/sarg");
+ if (TempDirPath[0]) {
+ append_to_path(tmp, sizeof(tmp), TempDirPath);
+ } else {
+ append_to_path(tmp, sizeof(tmp), "sargXXXXXX");
+ if (mkdtemp(tmp) == NULL) {
+ debuga(__FILE__,__LINE__,_("Failed to get a unique temporary directory name based on template \"%s\": %s\n"), tmp, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+ }
if (tmp[0]!='\0' && strncmp(outdir,tmp,strlen(tmp))==0) {
debuga(_("The output directory \"%s\" must be outside of the temporary directory \"%s\"\n"),outdir,tmp);
exit(EXIT_FAILURE);
}
+ atexit(CleanTemporaryDir);
if(email[0] == '\0' && OutputEmail[0] != '\0') strcpy(email,OutputEmail);
@@ -724,10 +739,7 @@ int main(int argc,char *argv[])
if (!KeepTempLog && unlink(denied_sort) && errno!=ENOENT)
debuga(_("Cannot delete \"%s\": %s\n"),denied_sort,strerror(errno));
}
-
- if(!KeepTempLog && strcmp(tmp,"/tmp") != 0) {
- unlinkdir(tmp,0);
- }
+ CleanTemporaryDir();
ip2name_cleanup();
free_hostalias();
@@ -758,7 +770,6 @@ int main(int argc,char *argv[])
exit(EXIT_SUCCESS);
}
-
static void getusers(const char *pwdfile, int debug)
{
FILE *fp_usr;
@@ -814,3 +825,10 @@ static void getusers(const char *pwdfile, int debug)
return;
}
+
+static void CleanTemporaryDir()
+{
+ if (!KeepTempLog && strcmp(tmp,"/tmp") != 0) {
+ unlinkdir(tmp,0);
+ }
+}
diff --git a/sarg.conf b/sarg.conf
index c579bea..9daef2f 100644
--- a/sarg.conf
+++ b/sarg.conf
@@ -126,6 +126,23 @@
#
#temporary_dir /tmp
+# TAG: temporary_dir_path
+# Path to append after the temporary_dir.
+# For historical reasons it used to be /sarg before v2.4. The full temporary
+# dir was, therefore, always the predicatble path /tmp/sarg. As it was considered
+# unsafe to use a predictable name in the world writable /tmp directory, the path
+# now used is a random unique name.
+# When this parameter is left empty, sarg uses a unique temporary path such as
+# sargXXXXXX where XXXXXX is replaced with a string to make the temporary dir unique
+# on the system.
+# The main drawback is that any temporary directory left over by a previous run of sarg
+# pollutes /tmp and may fill the disk up if sarg tends to crash often.
+# If you want to use a known fixed temporary path as it used to be prior to v2.4, you are
+# advised to set temporary_dir to /var/lib and set temporary_dir_path to /sarg. Sarg must
+# run as a user with the right to write to /var/lib/sarg.
+#
+#temporary_dir_path /sarg
+
# TAG: output_dir
# The reports will be saved in that directory
# sarg -o dir
diff --git a/util.c b/util.c
index d50f6f4..9bb7574 100644
--- a/util.c
+++ b/util.c
@@ -471,7 +471,7 @@ void makeTmpDir(const char *tmp)
* 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);
+ if (debug) debuga(__FILE__, __LINE__, _("Purging temporary directory \"%s\"\n"), tmp);
emptytmpdir(tmp);
}
}
@@ -2663,3 +2663,27 @@ int extract_address_mask(const char *buf,const char **text,unsigned char *ipv4,
ipv6[j++]=(unsigned short int)addr[i++];
return(3);
}
+
+void append_to_path(char *base_path, int base_path_size, const char *append)
+{
+ int length = strlen(base_path);
+ int append_length;
+
+ if (append[0] == '/') append++;
+ if (length > 0 && base_path[length-1] != '/') {
+ if (length+1 >= base_path_size) {
+ debuga(__FILE__, __LINE__, _("Path too long: "));
+ fprintf(stderr, "%s/%s", base_path, append);
+ exit(EXIT_FAILURE);
+ }
+ base_path[length++] = '/';
+ }
+ append_length = strlen(append);
+ if (length+append_length >= base_path_size) {
+ debuga(__FILE__, __LINE__, _("Path too long: "));
+ base_path[length] = '\0';
+ fprintf(stderr, "%s%s", base_path, append);
+ exit(EXIT_FAILURE);
+ }
+ strcpy(base_path + length, append);
+}
--
2.16.4
