CVE-2011-3364: GNOME NetworkManager Local Privilege Escalation

This is was a nice design flaw that could lead to local privilege escalation. However this is not such a high impact issue since it requires unprivileged local users having access to create new network connections in PolicyKit which is disabled by default.
As we can see here, this was reported by Matt McCutchen and we can find the buggy code in src/settings/plugins/ifcfg-rh/shvar.c source code file which is shown below.

/* create a new string with all necessary characters escaped.
 * caller must free returned string
 */
static const char escapees[] = "\"'\\$~`";              /* must be escaped */
static const char spaces[] = " \t|&;()<>";              /* only require "" */
char *
svEscape(const char *s) {
    char *new;
    int i, j, mangle = 0, space = 0;
    int newlen, slen;
    static int esclen, splen;

    if (!esclen) esclen = strlen(escapees);
    if (!splen) splen = strlen(spaces);
    slen = strlen(s);

    for (i = 0; i < slen; i++) {
        if (strchr(escapees, s[i])) mangle++;
        if (strchr(spaces, s[i])) space++;
    }
    if (!mangle && !space) return strdup(s);

    newlen = slen + mangle + 3; /* 3 is extra ""\0 */
    new = g_malloc0(newlen);
    if (!new) return NULL;

    j = 0;
    new[j++] = '"';
    for (i = 0; i < slen; i++) {
        if (strchr(escapees, s[i])) {
            new[j++] = '\\';
        }
        new[j++] = s[i];
    }
    new[j++] = '"';
    g_assert(j == slen + mangle + 2); /* j is the index of the '\0' */

    return new;
}

The above C routine is used to escape some characters. So, although it does check for some characters, more specifically the following:

- "
- '
- \
- $
- ~
- `
- \t
- |
- &
- ;
- (
- )
- <
- >

It does not check for new line character at all. Because of this, as Matt McCutchen mentioned, a user could create a new wired connection with a name of for example “name” and then rename it to something similar to:

'test\nUSERCTL=true\n/bin/bash'

Where newline entered via Ctrl-Shift-U, A (as we can read here). Now, if the following command is executed, it will spawn a root shell.

usernetctl test up

To fix this, another array was added in the shvar.c file which contains the newline sequence of characters.

 static const char escapees[] = "\"'\\$~`";		/* must be escaped */
 static const char spaces[] = " \t|&;()<>";		/* only require "" */
+static const char newlines[] = "\n\r";			/* only require "" */

Next, a new variable to count the newlines was added in svEscape() routine as you can see here:

 char *
 svEscape(const char *s) {
     char *new;
-    int i, j, mangle = 0, space = 0;
+    int i, j, mangle = 0, space = 0, newline = 0;
     int newlen, slen;
     static int esclen, splen;

And finally, the additional newlines checks were placed in the aforementioned function.

     for (i = 0; i < slen; i++) {
 	if (strchr(escapees, s[i])) mangle++;
 	if (strchr(spaces, s[i])) space++;
+	if (strchr(newlines, s[i])) newline++;
     }
-    if (!mangle && !space) return strdup(s);
+    if (!mangle && !space && !newline) return strdup(s);
 
-    newlen = slen + mangle + 3;	/* 3 is extra ""\0 */
+    newlen = slen + mangle - newline + 3;	/* 3 is extra ""\0 */
     new = g_malloc0(newlen);
     if (!new) return NULL;
 
     j = 0;
     new[j++] = '"';
     for (i = 0; i < slen; i++) {
+	if (strchr(newlines, s[i]))
+	    continue;
 	if (strchr(escapees, s[i])) {
 	    new[j++] = '\\';
 	}
 	new[j++] = s[i];
     }
     new[j++] = '"';
-    g_assert(j == slen + mangle + 2); /* j is the index of the '\0' */
+    g_assert(j == slen + mangle - newline + 2); /* j is the index of the '\0' */