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)

Reply via email to