a2ps builds a command line for file() containing an unescaped version of the file name, thus might call external programs described by the file name. Running a cronjob over a public writable directory a2ps-ing all files in it - or simply typing "a2ps *.txt" in /tmp - is therefore dangerous. Fix: Apply this patch (can be directly put into files/): How-To-Repeat: rpolzer:/tmp 7> touch 'x`echo>&2 42`' rpolzer:/tmp 8> a2ps -o /dev/null 'x`echo>&2 42`' 42 [x`echo>&2 42` (plain): 0 pages on 0 sheets] [Total: 0 pages on 0 sheets] saved into the file `/dev/null'
The patch has a minor bug: in out-of-memory conditions, it attempts to free the empty string "". Perhaps one should better return NULL in this case and let the other function return NULL, too, that is, change + if(!outp) + return ""; /* perhaps one should do better error handling here */ into + if(!outp) + return NULL; and + filename = shell_escape(filename); into + filename = shell_escape(filename); + if(filename == NULL) + return NULL; After that, IIRC @@ -144,11 +174,13 @@ must be changed into @@ -144,11 +174,15 @@ Then it should work even then.
The patch has a bug in the handling of a NULL return value of malloc. Here is a "better" patch: diff -ru ../a2ps-4.13.orig/src/select.c ./src/select.c --- ../a2ps-4.13.orig/src/select.c Wed Aug 18 16:32:09 2004 +++ ./src/select.c Wed Aug 18 16:49:12 2004 @@ -131,6 +131,36 @@ return 1; } +/* escapes the name of a file so that the shell groks it in 'single' q.marks. + The resulting pointer has to be free()ed when not longer used. */ +char * +shell_escape(const char *fn) +{ + size_t len = 0; + const char *inp; + char *retval, *outp; + + for(inp = fn; *inp; ++inp) + switch(*inp) + { + case '\'': len += 4; break; + default: len += 1; break; + } + + outp = retval = malloc(len + 1); + if(!outp) + return NULL; /* perhaps one should do better error handling here */ + for(inp = fn; *inp; ++inp) + switch(*inp) + { + case '\'': *outp++ = '\''; *outp++ = '\\'; *outp++ = '\'', *outp++ = '\''; break; + default: *outp++ = *inp; break; + } + *outp = 0; + + return retval; +} + /* What says file about the type of a file (result is malloc'd). NULL if could not be run. */ @@ -144,11 +174,15 @@ if (IS_EMPTY (job->file_command)) return NULL; + filename = shell_escape(filename); + if(filename == NULL) + return NULL; /* Call file(1) with the correct option */ - command = ALLOCA (char, (2 + command = ALLOCA (char, (4 + strlen (job->file_command) + ustrlen (filename))); - sprintf (command, "%s %s", job->file_command, (const char *) filename); + sprintf (command, "%s '%s'", job->file_command, (const char *) filename); + free(filename); message (msg_tool, (stderr, "Reading pipe: `%s'\n", command)); file_out = popen (command, "r");
Responsible Changed From-To: freebsd-ports-bugs->dinoex Over to maintainer.
State Changed From-To: open->closed committed, thanks.