Skip to content

Commit

Permalink
Sanitize process name for GUI notification helper
Browse files Browse the repository at this point in the history
As it turns out, the process name field /proc/PID/stat can
contain arbitrary characters. This is a problem, because
we call a notification helper, usually notify-send, using
system().

Aggressively strip all non-alphanumeric characters to fix
a shell code injection vulnerability.

Users who do not use GUI notifications (-n or -N) are
not affected.
  • Loading branch information
rfjakob committed Jul 6, 2018
1 parent c91dc9e commit ab79aa3
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
3 changes: 3 additions & 0 deletions kill.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#define BADNESS_AVOID -300

extern int enable_debug;
void sanitize(char* s);

struct procinfo {
int oom_score;
Expand Down Expand Up @@ -219,6 +220,8 @@ static void userspace_kill(DIR* procdir, int sig, int ignore_oom_score_adj,
// that there is enough memory to spawn the notification helper.
if (sig != 0) {
char notif_args[PATH_MAX + 1000];
// maybe_notify() calls system(). We must sanitize the strings we pass.
sanitize(victim_name);
snprintf(notif_args, sizeof(notif_args), "-i dialog-warning 'earlyoom' 'Killing process %d %s'", victim_pid, victim_name);
maybe_notify(notif_command, notif_args);
}
Expand Down
16 changes: 16 additions & 0 deletions sanitize.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// SPDX-License-Identifier: MIT

/* sanitize replaces everything in string "s" that is not [a-zA-Z0-9]
* with an underscore. The resulting string is safe to pass to a shell.
*/
void sanitize(char* s)
{
char c;
for (int i = 0; s[i] != 0; i++) {
c = s[i];
if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9')) {
continue;
}
s[i] = '_';
}
}

0 comments on commit ab79aa3

Please sign in to comment.