I have been playing around with this a little and believe I have come up with a workable solution. The attached patch causes the passed in format string to be dumbed down so that we only translate instances of %d into the PID and \n into newline characters. Everything else is treated as a literal part of the string.
This effectively should neutralize any use of %s %c %f etc to cause a segfault or dump memory. (Hopefully.) It means the output string cannot have fancy formatting, but that seems like a job better suited to sed or awk anyway. I have tested this on a handful of formatted strings and it seems to fix the original problem. I'm open to improvements or fixing other corner cases I may have missed. Assuming people are more or less happy with this fix it will get applied upstream. Jesse
diff --git a/man/pidof.8 b/man/pidof.8 index a4295b9..4e83492 100644 --- a/man/pidof.8 +++ b/man/pidof.8 @@ -69,8 +69,10 @@ Tells \fIpidof\fP to omit processes with that process id. The special pid \fB%PPID\fP can be used to name the parent process of the \fIpidof\fP program, in other words the calling shell or shell script. .IP "-f \fIformat\fP" -Tells \fIpidof\fP to format the process ids in the given \fIprintf\fP style. +Tells \fIpidof\fP to format the process IDs in the given \fIprintf\fP style string. For example \fB" -p%d"\fP is useful for \fIstrace\fP. +The "%d" symbol is used as a place holder for the PID to be printed. A "\\n" can +be used to cause a newline to be printed. For example \fB" %d\\n"\fP. .SH "EXIT STATUS" .TP .B 0 diff --git a/src/killall5.c b/src/killall5.c index 8f2ad50..375b4a7 100644 --- a/src/killall5.c +++ b/src/killall5.c @@ -985,6 +985,66 @@ void nsyslog(int pri, char *fmt, ...) #define PIDOF_NETFS 0x04 #define PIDOF_QUIET 0x08 + +/* Replace elements (from) of the original string + with new elements (to). + Returns the new string on success or NULL on failure. + Free the returnedstring after use. +*/ +char *Replace_String(char *from, char *to, char *original) +{ + int from_length, to_length; + int source_length, destination_length; + int replace_count = 0; + char *replace_position; + char *destination_string; + char *source_position, *destination_position; + + if ( (! from) || (! to) || (! original) ) + return NULL; + + from_length = strlen(from); + to_length = strlen(to); + source_length = strlen(original); + replace_position = strstr(original, from); + /* There is nothing to replace, return original string */ + if (! replace_position) + return strdup(original); + /* count number of times we need to perform replacement */ + while (replace_position) + { + replace_count++; + replace_position++; + replace_position = strstr(replace_position, from); + } + /* calculate length and allocate the new string */ + destination_length = source_length + ( (to_length - from_length) * replace_count); + destination_string = calloc(destination_length, sizeof(char)); + if (! destination_string) + return NULL; + + /* Copy source string up to the part we need to replace. Then jump over the replaced bit */ + source_position = original; + destination_position = destination_string; + replace_position = strstr(original, from); + while (replace_position) + { + for ( ; source_position < replace_position; source_position++) + { + destination_position[0] = source_position[0]; + destination_position++; + } + strcat(destination_position, to); + source_position += from_length; + destination_position += to_length; + replace_position = strstr(source_position, from); + } + /* Replaced all the items, now copy the tail end of the original string */ + strcat(destination_position, source_position); + return destination_string; +} + + /* * Pidof functionality. */ @@ -1119,7 +1179,22 @@ int main_pidof(int argc, char **argv) if ( ~flags & PIDOF_QUIET ) { if (format) - printf(format, p->pid); + { + char *show_string, *final_string; + char my_pid[32]; + snprintf(my_pid, 32, "%d", p->pid); + show_string = Replace_String("%d", my_pid, format); + final_string = Replace_String("\\n", "\n", show_string); + if (show_string) free(show_string); + if (final_string) + { + printf("%s", final_string); + free(final_string); + } + else + fprintf(stderr, "Cannot handle format provided by -f\n"); + /* printf(format, p->pid); */ + } else { if (! first)