Bug#924792: pidof: unsanitized user input makes pidof crash
Jesse Smith
jsmith at resonatingmedia.com
Mon Mar 18 20:10:36 GMT 2019
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
-------------- next part --------------
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)
More information about the Debian-init-diversity
mailing list