Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 4 May 2004 17:39:07 -0700
From:      "Crist J. Clark" <cristjc@comcast.net>
To:        freebsd-security@freebsd.org
Subject:   Re: ctags(1) command execution vulnerability
Message-ID:  <20040505003907.GA80906@blossom.cjclark.org>
In-Reply-To: <20040504054909.GA3119@lame.novel.ru>
References:  <20040504054909.GA3119@lame.novel.ru>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, May 04, 2004 at 09:49:09AM +0400, Roman Bogorodskiy wrote:
> Hello,
> 
> 	ctags(1) uses external application sort(1) for sorting the tags file.
> It calls it via system(3) function. 
> 
> Look at the /usr/src/usr.bin/ctags/ctags.c file, there are such lines
> here: 
> 
> if (uflag) {
> 	(void)asprintf(&cmd, "sort -o %s %s",
> 	    outfile, outfile);
> 	if (cmd == NULL)
> 		err(1, "out of space");
> 	system(cmd);
> 	free(cmd);
> 	cmd = NULL;
> }

Ick. There's another system(3) call above this one too,

				for (step = 0; step < argc; step++) {
					(void)asprintf(&cmd,
					    "mv %s OTAGS; fgrep -v '\t%s\t' OTAGS >%s; rm OTAGS",
					    outfile, argv[step], outfile);
					if (cmd == NULL)
						err(1, "out of space");
					system(cmd);
					free(cmd);
					cmd = NULL;
				}

Also bad.

> This code will be executed when "-u" arg was given. So, if we'll execute 
> ctags in a such way:
> 
> ctags -u -f ';echo hi' *.c
> 
> we get the following:
> 
> Syntax error: ";" unexpected
> sort: option requires an argument -- o
> Try `sort --help' for more information.
> hi
> hi

The two 'hi's are from each system(3) call.

> Solution:

As has been pointed out, the problem here is user supplied data to a system(3)
call that we really cannot sanitize without filtering a lot of valid file names.
The Right Thing is to get rid of system(3).

This seems to work. Fixing the sort is trivial. Adding the regex checks to the
program adds a little complexity, but not a lot. Anyone who actually uses 
ctags(1) want to try them out some more to see if they hold up?


Index: ctags.c
===================================================================
RCS file: /export/freebsd/ncvs/src/usr.bin/ctags/ctags.c,v
retrieving revision 1.18
diff -u -r1.18 ctags.c
--- ctags.c	28 Jul 2002 15:50:38 -0000	1.18
+++ ctags.c	5 May 2004 00:27:09 -0000
@@ -44,11 +44,13 @@
 #endif
 
 #include <sys/cdefs.h>
+#include <sys/types.h>
 __FBSDID("$FreeBSD: src/usr.bin/ctags/ctags.c,v 1.18 2002/07/28 15:50:38 dwmalone Exp $");
 
 #include <err.h>
 #include <limits.h>
 #include <locale.h>
+#include <regex.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -164,16 +166,37 @@
 			put_entries(head);
 		else {
 			if (uflag) {
+				FILE *oldf;
+				regex_t *regx;
+
+				if ((oldf = fopen(outfile, "r")) == NULL)
+					err(1, "opening %s", outfile);
+				if (unlink(outfile))
+					err(1, "unlinking %s", outfile);
+				if ((outf = fopen(outfile, "w")) == NULL)
+					err(1, "recreating %s", outfile);
+				if ((regx = calloc(argc, sizeof(regex_t))) == NULL)
+					err(1, "RE alloc");
 				for (step = 0; step < argc; step++) {
-					(void)asprintf(&cmd,
-					    "mv %s OTAGS; fgrep -v '\t%s\t' OTAGS >%s; rm OTAGS",
-					    outfile, argv[step], outfile);
-					if (cmd == NULL)
-						err(1, "out of space");
-					system(cmd);
-					free(cmd);
-					cmd = NULL;
+					(void)strcpy(lbuf, "\t");
+					(void)strlcat(lbuf, argv[step], LINE_MAX);
+					(void)strlcat(lbuf, "\t", LINE_MAX);
+					if (regcomp(regx + step, lbuf,
+					    REG_NOSPEC))
+						warn("RE compilation failed");
+				}
+nextline:
+				while (fgets(lbuf, LINE_MAX, oldf)) {
+					for (step = 0; step < argc; step++)
+						if (regexec(regx + step,
+						    lbuf, 0, NULL, 0) == 0)
+							goto nextline;
+					fputs(lbuf, outf);
 				}
+				for (step = 0; step < argc; step++)
+					regfree(regx + step);
+				fclose(oldf);
+				fclose(outf);
 				++aflag;
 			}
 			if (!(outf = fopen(outfile, aflag ? "a" : "w")))
@@ -181,13 +204,15 @@
 			put_entries(head);
 			(void)fclose(outf);
 			if (uflag) {
-				(void)asprintf(&cmd, "sort -o %s %s",
-				    outfile, outfile);
-				if (cmd == NULL)
-					err(1, "out of space");
-				system(cmd);
-				free(cmd);
-				cmd = NULL;
+				pid_t pid;
+				if ((pid = fork()) == -1)
+					err(1, "fork failed");
+				else if (pid == 0) {
+					execlp("sort", "sort", "-o", outfile,
+					    outfile, NULL);
+					/* NOT REACHED */
+					err(1, "exec of sort failed");
+				}
 			}
 		}
 	}
-- 
Crist J. Clark                     |     cjclark@alum.mit.edu
                                   |     cjclark@jhu.edu
http://people.freebsd.org/~cjc/    |     cjc@freebsd.org



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040505003907.GA80906>