From: Frédéric Marchal <fmarchal@users.sourceforge.net>
Date: Sat, 18 Jun 2011 10:56:40 +0000 (+0000)
Subject: Protect the sort commands against buffer overflow
X-Git-Tag: v2.3.2~54
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=78eeb33f04aacd8e739bd1938323dce5e13cb9a7;p=thirdparty%2Fsarg.git

Protect the sort commands against buffer overflow

The external sort commands are build by snprintf instead of sprintf
to guard against buffer overflows.
---

diff --git a/email.c b/email.c
index c600f73..c103da5 100644
--- a/email.c
+++ b/email.c
@@ -130,7 +130,10 @@ int geramail(const char *dirname, int debug, const char *outdir, const char *ema
 	}
 #endif
 
-	sprintf(csort,"sort -n -T \"%s\" -t \"\t\" -r -k 2,2 -o \"%s\" \"%s\"", TempDir, top1, top2);
+	if (snprintf(csort,sizeof(csort),"sort -n -T \"%s\" -t \"\t\" -r -k 2,2 -o \"%s\" \"%s\"", TempDir, top1, top2)>=sizeof(csort)) {
+		debuga(_("Sort command too long when sorting file \"%s\" to \"%s\"\n"),top2,top1);
+		exit(EXIT_FAILURE);
+	}
 	cstatus=system(csort);
 	if (!WIFEXITED(cstatus) || WEXITSTATUS(cstatus)) {
 		debuga(_("sort command return status %d\n"),WEXITSTATUS(cstatus));
diff --git a/html.c b/html.c
index 2c3a496..545733d 100644
--- a/html.c
+++ b/html.c
@@ -400,7 +400,10 @@ void htmlrel(void)
 				fclose(fp_ip);
 				fclose(fp_ip2);
 
-				sprintf(csort,"sort -n -t \"\t\" -T \"%s\" -k 1,1 -k 2,2 -o \"%s\" \"%s\"",tmp,tmp3,tmp2);
+				if (snprintf(csort,sizeof(csort),"sort -n -t \"\t\" -T \"%s\" -k 1,1 -k 2,2 -o \"%s\" \"%s\"",tmp,tmp3,tmp2)>=sizeof(csort)) {
+					debuga(_("Sort command too long when sorting file \"%s\" to \"%s\"\n"),tmp2,tmp3);
+					exit(EXIT_FAILURE);
+				}
 				cstatus=system(csort);
 				if (!WIFEXITED(cstatus) || WEXITSTATUS(cstatus)) {
 					debuga(_("sort command return status %d\n"),WEXITSTATUS(cstatus));
diff --git a/log.c b/log.c
index 6dfdd0f..59aca0d 100644
--- a/log.c
+++ b/log.c
@@ -1661,7 +1661,10 @@ int main(int argc,char *argv[])
 	}
 
 	if(DataFile[0] == '\0' && (ReportType & REPORT_TYPE_DENIED) != 0) {
-		sprintf(csort,"sort -T \"%s\" -t \"\t\" -k 3,3 -k 5,5 -o \"%s\" \"%s\"",tmp,denied_sort,denied_unsort);
+		if (snprintf(csort,sizeof(csort),"sort -T \"%s\" -t \"\t\" -k 3,3 -k 5,5 -o \"%s\" \"%s\"",tmp,denied_sort,denied_unsort)>=sizeof(csort)) {
+			debuga(_("Sort command too long when sorting file \"%s\" to \"%s\"\n"),denied_unsort,denied_sort);
+			exit(EXIT_FAILURE);
+		}
 		cstatus=system(csort);
 		if (!WIFEXITED(cstatus) || WEXITSTATUS(cstatus)) {
 			debuga(_("sort command return status %d\n"),WEXITSTATUS(cstatus));
diff --git a/realtime.c b/realtime.c
index 907c99b..7c77bc7 100755
--- a/realtime.c
+++ b/realtime.c
@@ -79,7 +79,10 @@ static void getlog(void)
 	fclose(tmp);
 	longline_destroy(&line);
 
-	sprintf(cmd,"sort -t \"\t\" -r -k 1,1 -k 2,2 -o \"%s\" \"%s\"",template2,template1);
+	if (snprintf(cmd,sizeof(cmd),"sort -t \"\t\" -r -k 1,1 -k 2,2 -o \"%s\" \"%s\"",template2,template1)>=sizeof(cmd)) {
+		debuga(_("Sort command too long when sorting file \"%s\" to \"%s\"\n"),template1,template2);
+		exit(EXIT_FAILURE);
+	}
 	cstatus=system(cmd);
 	if (!WIFEXITED(cstatus) || WEXITSTATUS(cstatus)) {
 		debuga(_("sort command return status %d\n"),WEXITSTATUS(cstatus));
diff --git a/siteuser.c b/siteuser.c
index d5d6216..d64af48 100644
--- a/siteuser.c
+++ b/siteuser.c
@@ -59,7 +59,10 @@ void siteuser(void)
 	sprintf(general2,"%s/sarg-general2",outdirname);
 	sprintf(report,"%s/siteuser.html",outdirname);
 
-	sprintf(csort,"sort -t \"\t\" -k 4,4 -k 1,1 -o \"%s\" \"%s\"",general2,general);
+	if (snprintf(csort,sizeof(csort),"sort -t \"\t\" -k 4,4 -k 1,1 -o \"%s\" \"%s\"",general2,general)>=sizeof(csort)) {
+		debuga(_("Sort command too long when sorting file \"%s\" to \"%s\"\n"),general,general2);
+		exit(EXIT_FAILURE);
+	}
 	cstatus=system(csort);
 	if (!WIFEXITED(cstatus) || WEXITSTATUS(cstatus)) {
 		debuga(_("sort command return status %d\n"),WEXITSTATUS(cstatus));
diff --git a/sort.c b/sort.c
index b3989af..fb67e76 100644
--- a/sort.c
+++ b/sort.c
@@ -79,7 +79,10 @@ void tmpsort(void)
 			debuga(_("Sorting file: %s\n"),arqou);
 		}
 
-		sprintf(csort,"sort -n -T \"%s\" -t \"\t\" %s -k %s -k %s -k %s -o \"%s\" \"%s\"",tmp,order,field1,field2,field3,arqou,arqin);
+		if (snprintf(csort,sizeof(csort),"sort -n -T \"%s\" -t \"\t\" %s -k %s -k %s -k %s -o \"%s\" \"%s\"",tmp,order,field1,field2,field3,arqou,arqin)>=sizeof(csort)) {
+			debuga(_("Sort command too long when sorting file \"%s\" to \"%s\"\n"),arqin,arqou);
+			exit(EXIT_FAILURE);
+		}
 		cstatus=system(csort);
 		if (!WIFEXITED(cstatus) || WEXITSTATUS(cstatus)) {
 			debuga(_("sort command return status %d\n"),WEXITSTATUS(cstatus));
diff --git a/squidguard_log.c b/squidguard_log.c
index 2d01a66..cc602b7 100644
--- a/squidguard_log.c
+++ b/squidguard_log.c
@@ -379,7 +379,10 @@ void squidguard_log(void)
 		debuga(_("Sorting file: %s\n"),guard_ou);
 	}
 
-	sprintf(tmp6,"sort -t \"\t\" -k 1,1 -k 2,2 -k 4,4 \"%s\" -o \"%s\"",guard_in, guard_ou);
+	if (snprintf(tmp6,sizeof(tmp6),"sort -t \"\t\" -k 1,1 -k 2,2 -k 4,4 \"%s\" -o \"%s\"",guard_in, guard_ou)>=sizeof(tmp6)) {
+		debuga(_("Sort command too long when sorting file \"%s\" to \"%s\"\n"),guard_in,guard_ou);
+		exit(EXIT_FAILURE);
+	}
 	cstatus=system(tmp6);
 	if (!WIFEXITED(cstatus) || WEXITSTATUS(cstatus)) {
 		debuga(_("sort command return status %d\n"),WEXITSTATUS(cstatus));
diff --git a/topsites.c b/topsites.c
index dd14f6a..60d2c09 100644
--- a/topsites.c
+++ b/topsites.c
@@ -79,7 +79,10 @@ void topsites(void)
 	else
 		sprintf(report,"%s/topsites.html",outdirname);
 
-	sprintf(csort,"sort -t \"\t\" -k 4,4 -o \"%s\" \"%s\"",general2,general);
+	if (snprintf(csort,sizeof(csort),"sort -t \"\t\" -k 4,4 -o \"%s\" \"%s\"",general2,general)>=sizeof(csort)) {
+		debuga(_("Sort command too long when sorting file \"%s\" to \"%s\"\n"),general,general2);
+		exit(EXIT_FAILURE);
+	}
 	cstatus=system(csort);
 	if (!WIFEXITED(cstatus) || WEXITSTATUS(cstatus)) {
 		debuga(_("sort command return status %d\n"),WEXITSTATUS(cstatus));
@@ -192,7 +195,10 @@ void topsites(void)
 		sortt="";
 	}
 
-	sprintf(csort,"sort -t \"\t\" %s -n %s -o \"%s\" \"%s\"",sortt,sortf,sites,general3);
+	if (snprintf(csort,sizeof(csort),"sort -t \"\t\" %s -n %s -o \"%s\" \"%s\"",sortt,sortf,sites,general3)>=sizeof(csort)) {
+		debuga(_("Sort command too long when sorting file \"%s\" to \"%s\"\n"),general3,sites);
+		exit(EXIT_FAILURE);
+	}
 	cstatus=system(csort);
 	if (!WIFEXITED(cstatus) || WEXITSTATUS(cstatus)) {
 		debuga(_("sort command return status %d\n"),WEXITSTATUS(cstatus));
diff --git a/topuser.c b/topuser.c
index 463a482..4394eec 100644
--- a/topuser.c
+++ b/topuser.c
@@ -167,7 +167,10 @@ void topuser(void)
 	}
 
 	snprintf(top1,sizeof(top1),"%s/top",outdirname);
-	sprintf(csort,"sort -T \"%s\" -t \"\t\" %s %s -o \"%s\" \"%s\"", tmp, order, sfield, top1, top2);
+	if (snprintf(csort,sizeof(csort),"sort -T \"%s\" -t \"\t\" %s %s -o \"%s\" \"%s\"", tmp, order, sfield, top1, top2)>=sizeof(csort)) {
+		debuga(_("Sort command too long when sorting file \"%s\" to \"%s\"\n"),top2,top1);
+		exit(EXIT_FAILURE);
+	}
 	cstatus=system(csort);
 	if (!WIFEXITED(cstatus) || WEXITSTATUS(cstatus)) {
 		debuga(_("sort command return status %d\n"),WEXITSTATUS(cstatus));
diff --git a/useragent.c b/useragent.c
index c3f6f9a..d6e5f73 100644
--- a/useragent.c
+++ b/useragent.c
@@ -132,7 +132,10 @@ void useragent(void)
 		debuga(_("Sorting file: %s\n"),tmp2);
 	}
 
-	sprintf(csort,"sort -n -t \"\t\" -k 3,3 -k 2,2 -k 1,1 -o \"%s\" \"%s\"",tmp2,tmp3);
+	if (snprintf(csort,sizeof(csort),"sort -n -t \"\t\" -k 3,3 -k 2,2 -k 1,1 -o \"%s\" \"%s\"",tmp2,tmp3)>=sizeof(csort)) {
+		debuga(_("Sort command too long when sorting file \"%s\" to \"%s\"\n"),tmp2,tmp3);
+		exit(EXIT_FAILURE);
+	}
 	cstatus=system(csort);
 	if (!WIFEXITED(cstatus) || WEXITSTATUS(cstatus)) {
 		debuga(_("sort command return status %d\n"),WEXITSTATUS(cstatus));
@@ -212,7 +215,10 @@ void useragent(void)
 	fputs("</table>\n",fp_ht);
 	fclose(fp_in);
 
-	sprintf(csort,"sort -t \"\t\" -k 2,2 -o \"%s\" \"%s\"",tmp3,tmp2);
+	if (snprintf(csort,sizeof(csort),"sort -t \"\t\" -k 2,2 -o \"%s\" \"%s\"",tmp3,tmp2)>=sizeof(csort)) {
+		debuga(_("Sort command too long when sorting file \"%s\" to \"%s\"\n"),tmp2,tmp3);
+		exit(EXIT_FAILURE);
+	}
 	cstatus=system(csort);
 	if (!WIFEXITED(cstatus) || WEXITSTATUS(cstatus)) {
 		debuga(_("sort command return status %d\n"),WEXITSTATUS(cstatus));
@@ -278,7 +284,10 @@ void useragent(void)
 		exit(EXIT_FAILURE);
 	}
 
-	sprintf(csort,"sort -n -r -k 1,1 -o \"%s\" \"%s\"",tmp3,tmp2);
+	if (snprintf(csort,sizeof(csort),"sort -n -r -k 1,1 -o \"%s\" \"%s\"",tmp3,tmp2)>=sizeof(csort)) {
+		debuga(_("Sort command too long when sorting file \"%s\" to \"%s\"\n"),tmp2,tmp3);
+		exit(EXIT_FAILURE);
+	}
 	cstatus=system(csort);
 	if (!WIFEXITED(cstatus) || WEXITSTATUS(cstatus)) {
 		debuga(_("sort command return status %d\n"),WEXITSTATUS(cstatus));