On Wed, Mar 15, 2006 at 16:58:10 +1100, Brian May wrote:
> This could be a security issue, if you can run pstotext with an arbitrary
> filename (eg. via swish++ running on some untrusted source).
pstotext currently popen(3)s a command containing a filename supplied by its
caller. The only way to secure it against nasty filenames would be to
rewrite it not to use popen(3). The attached patch seems to do the trick but
should be reviewed - I'm not sure I've done the fork'n'pipe completely
right; there may be some cleanup missing and it breaks the VMS build (though
I doubt someone cares about that). Oh well, it's a start.
Ray
--
Sexual paranoia: did I once unknowingly sleep with THEM?
diff -ru ../pstotext-1.9/main.c pstotext-1.9/main.c
--- ../pstotext-1.9/main.c 2005-07-24 18:56:11.000000000 +0200
+++ pstotext-1.9/main.c 2006-03-15 23:02:20.000000000 +0100
@@ -166,7 +166,13 @@
static int do_it(char *path) {
/* If "path" is NULL, then "stdin" should be processed. */
- char *gs_cmdline;
+ char *gs_argv[32];
+ int gs_argc=0;
+#ifdef DEBUG
+ int i;
+#endif
+ int fd[2];
+ pid_t p;
char *input;
int status;
char norotate[] = "";
@@ -216,32 +222,29 @@
cleanup();
exit(1);
}
- strcpy(input, "-- '"); strcat(input, path); strcat(input, "'");
- }
-
- gs_cmdline = (char*)malloc(strlen(gs_cmd)+strlen(rotate_path)+
- strlen(ocr_path) + strlen(input) + 128);
-
- if (gs_cmdline == NULL) {
- fprintf(stderr, "No memory available\n");
- cleanup();
- exit(1);
+ strcpy(input, path);
}
- sprintf(
- gs_cmdline,
-#ifdef VMS
- "%s -r72 \"-dNODISPLAY\" \"-dFIXEDMEDIA\" \"-dDELAYBIND\"
\"-dWRITESYSTEMDICT\" %s \"-dNOPAUSE\" \"-dSAFER\" %s %s %s",
-#else
- "%s -r72 -dNODISPLAY -dFIXEDMEDIA -dDELAYBIND -dWRITESYSTEMDICT %s
-dNOPAUSE -dSAFER %s %s %s",
-#endif
- gs_cmd,
- (debug ? "" : "-q"),
- rotate_path,
- ocr_path,
- input
- );
- if (debug) fprintf(stderr, "%s\n", gs_cmdline);
+ gs_argv[gs_argc++] = "gs";
+ gs_argv[gs_argc++] = "-r72";
+ gs_argv[gs_argc++] = "-dNODISPLAY";
+ gs_argv[gs_argc++] = "-dFIXEDMEDIA";
+ gs_argv[gs_argc++] = "-dDELAYBIND";
+ gs_argv[gs_argc++] = "-dWRITESYSTEMDICT";
+ if (!debug) {
+ gs_argv[gs_argc++] = "-q";
+ }
+ gs_argv[gs_argc++] = "-dNOPAUSE";
+ gs_argv[gs_argc++] = "-dSAFER";
+ if (rotate_path && strcmp(rotate_path, "")) {
+ gs_argv[gs_argc++] = rotate_path;
+ }
+ if (ocr_path && strcmp(ocr_path, "")) {
+ gs_argv[gs_argc++] = ocr_path;
+ }
+ gs_argv[gs_argc++] = "--";
+ gs_argv[gs_argc++] = input;
+ gs_argv[gs_argc++] = NULL;
#ifdef VMS
cmdfile = tempnam("SYS$SCRATCH:","PS2TGS");
gsoutfile = tempnam("SYS$SCRATCH:","GSRES");
@@ -259,8 +262,28 @@
exit(1);
}
#else
- gs = popen(gs_cmdline, "r");
- if (gs==0) {perror(cmd); exit(1);}
+#ifdef DEBUG
+ for (i=0; i < gs_argc; i++) {
+ fprintf(stderr, "%i: ->%s<-\n", i, gs_argv[i]);
+ }
+#endif
+ if (pipe(fd)) {
+ perror("pipe failed: "); exit(1);
+ };
+ p = fork();
+ if (p == -1) {
+ perror("fork failed: "); exit(1);
+ }
+ if (p == 0) { /* child */
+ close(fd[0]);
+ dup2(fd[1], 1); /* Redirect stdout into pipe to parent */
+ execvp("/usr/bin/gs", gs_argv);
+ perror("execvp: "); exit(1);
+ } else { /* parent */
+ close(fd[1]);
+ gs = fdopen(fd[0], "r");
+ /* FIXME: do we need to arrange for reaping the child? */
+ }
#endif
status = pstotextInit(&instance);
if (status!=0) {
