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 | [email protected] | [email protected] http://people.freebsd.org/~cjc/ | [email protected] _______________________________________________ [email protected] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-security To unsubscribe, send any mail to "[email protected]"