From f6fd14e14cc3ecd621e863c1fb5a58c54ba1f2d7 Mon Sep 17 00:00:00 2001 From: Michael Ikey Doherty Date: Thu, 28 Nov 2013 09:43:21 +0000 Subject: [PATCH 1/3] daemon/verify-pam: Bail if popen() fails If we fail to run popen() to determine the last user for any reason, be it I/O error or even memory allocation, we should immediately exit the method, as we can no longer safely continue. --- daemon/verify-pam.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/daemon/verify-pam.c b/daemon/verify-pam.c index 1799de1..0e28b28 100644 --- a/daemon/verify-pam.c +++ b/daemon/verify-pam.c @@ -458,8 +458,14 @@ create_pamh (MdmDisplay *d, mdm_debug("mdm_verify_user: Automatic/Timed login detected, not presetting user."); } else { + FILE *fp = NULL; char last_username[255]; - FILE *fp = popen("last -w | grep tty | head -1 | awk {'print $1;'}", "r"); + + fp = popen("last -w | grep tty | head -1 | awk {'print $1;'}", "r"); + if (!fp) { + mdm_debug("mdm_verify_user: Failed to determine last user (popen())"); + return FALSE; + } fscanf(fp, "%s", last_username); pclose(fp); if (last_username != NULL && strcmp (last_username, "") != 0) { From e5b27f539d76673fd9b47594903b3ddc6837b938 Mon Sep 17 00:00:00 2001 From: Michael Ikey Doherty Date: Thu, 28 Nov 2013 09:59:56 +0000 Subject: [PATCH 2/3] gui/mdmwebkit: Safely handle potential popen() failure If we fail to run popen() to obtain the lsb description for any reason, we should attempt to work around the situation (much like in the previous commit). This commit also switches the fixed length within the fgets call to use sizeof, ensuring we always have the correct size as specified in the declaration of our lsb_description array. --- gui/mdmwebkit.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/gui/mdmwebkit.c b/gui/mdmwebkit.c index 9dfa2ea..af8b2ce 100644 --- a/gui/mdmwebkit.c +++ b/gui/mdmwebkit.c @@ -876,6 +876,8 @@ static gboolean key_press_event (GtkWidget *widget, GdkEventKey *key, gpointer d static void webkit_init (void) { GError *error; char *html; + FILE *fp = NULL; + char lsb_description[255]; gsize file_length; gchar * theme_name = mdm_config_get_string (MDM_KEY_HTML_THEME); gchar * theme_dir = g_strdup_printf("file:///usr/share/mdm/html-themes/%s/", theme_name); @@ -913,10 +915,14 @@ static void webkit_init (void) { } - char lsb_description[255]; - FILE *fp = popen("lsb_release -d -s", "r"); - fgets(lsb_description, 255, fp); - pclose(fp); + fp = popen("lsb_release -d -s", "r"); + if (fp) { + fgets(lsb_description, sizeof(lsb_description), fp); + pclose(fp); + } else { + /* popen failed, use blank lsb_description */ + lsb_description = ""; + } html = str_replace(html, "$lsb_description", lsb_description); html = str_replace(html, "$login_label", html_encode(_("Login"))); From 00505e0db5dea713cba76ba60b0cab7da347af21 Mon Sep 17 00:00:00 2001 From: Michael Ikey Doherty Date: Thu, 28 Nov 2013 10:47:06 +0000 Subject: [PATCH 3/3] gui/mdmwebkit: Ensure we cleanup allocated strings This commit also drops the redundant replacements in html_encode by making use of g_markup_printf_escaped, which will safely escape non displayable markup characters such as < > &, etc. using their HTML-safe equivalents, such as &. --- gui/mdmwebkit.c | 59 +++++++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/gui/mdmwebkit.c b/gui/mdmwebkit.c index af8b2ce..49c5797 100644 --- a/gui/mdmwebkit.c +++ b/gui/mdmwebkit.c @@ -121,26 +121,31 @@ static void check_for_displays (void) { g_strfreev (vec); } -static gchar * str_replace(const char *string, const char *delimiter, const char *replacement) { +static gchar * str_replace(gchar **string, const gchar *delimiter, gchar *replacement, gboolean free_replace) { gchar **split; gchar *ret; g_return_val_if_fail(string != NULL, NULL); g_return_val_if_fail(delimiter != NULL, NULL); g_return_val_if_fail(replacement != NULL, NULL); - split = g_strsplit(string, delimiter, 0); + split = g_strsplit(*string, delimiter, 0); + g_free(*string); + *string = NULL; + ret = g_strjoinv(replacement, split); g_strfreev(split); + + /* If the replacement is allocated, we should now free the replacement string */ + if (free_replace) + g_free(replacement); + return ret; } static char * html_encode(const char *string) { - char * ret; - ret = str_replace(string, "'", "'"); - ret = str_replace(ret, "\"", """); - ret = str_replace(ret, ";", ";"); - ret = str_replace(ret, "<", "<"); - ret = str_replace(ret, ">", ">"); + gchar *ret = NULL; + + ret = g_markup_printf_escaped(string); return ret; } @@ -924,25 +929,25 @@ static void webkit_init (void) { lsb_description = ""; } - html = str_replace(html, "$lsb_description", lsb_description); - html = str_replace(html, "$login_label", html_encode(_("Login"))); - html = str_replace(html, "$ok_label", html_encode(_("OK"))); - html = str_replace(html, "$cancel_label", html_encode(_("Cancel"))); - html = str_replace(html, "$enter_your_username_label", html_encode(_("Please enter your username"))); - html = str_replace(html, "$enter_your_password_label", html_encode(_("Please enter your password"))); - html = str_replace(html, "$hostname", g_get_host_name()); - html = str_replace(html, "$shutdown", html_encode(_("Shutdown"))); - html = str_replace(html, "$suspend", html_encode(_("Suspend"))); - html = str_replace(html, "$quit", html_encode(_("Quit"))); - html = str_replace(html, "$restart", html_encode(_("Restart"))); - html = str_replace(html, "$session", html_encode(_("Session"))); - html = str_replace(html, "$selectsession", html_encode(_("Select a session"))); - html = str_replace(html, "$defaultsession", html_encode(_("Default session"))); - html = str_replace(html, "$language", html_encode(_("Language"))); - html = str_replace(html, "$selectlanguage", html_encode(_("Select a language"))); - html = str_replace(html, "$areyousuretoquit", html_encode(_("Are you sure you want to quit?"))); - html = str_replace(html, "$close", html_encode(_("Close"))); - html = str_replace(html, "$locale", g_strdup (setlocale (LC_MESSAGES, NULL))); + html = str_replace(&html, "$lsb_description", lsb_description, FALSE); + html = str_replace(&html, "$login_label", html_encode(_("Login")), TRUE); + html = str_replace(&html, "$ok_label", html_encode(_("OK")), TRUE); + html = str_replace(&html, "$cancel_label", html_encode(_("Cancel")), TRUE); + html = str_replace(&html, "$enter_your_username_label", html_encode(_("Please enter your username")), TRUE); + html = str_replace(&html, "$enter_your_password_label", html_encode(_("Please enter your password")), TRUE); + html = str_replace(&html, "$hostname", g_get_host_name(), FALSE); + html = str_replace(&html, "$shutdown", html_encode(_("Shutdown")), TRUE); + html = str_replace(&html, "$suspend", html_encode(_("Suspend")), TRUE); + html = str_replace(&html, "$quit", html_encode(_("Quit")), TRUE); + html = str_replace(&html, "$restart", html_encode(_("Restart")), TRUE); + html = str_replace(&html, "$session", html_encode(_("Session")), TRUE); + html = str_replace(&html, "$selectsession", html_encode(_("Select a session")), TRUE); + html = str_replace(&html, "$defaultsession", html_encode(_("Default session")), TRUE); + html = str_replace(&html, "$language", html_encode(_("Language")), TRUE); + html = str_replace(&html, "$selectlanguage", html_encode(_("Select a language")), TRUE); + html = str_replace(&html, "$areyousuretoquit", html_encode(_("Are you sure you want to quit?")), TRUE); + html = str_replace(&html, "$close", html_encode(_("Close")), TRUE); + html = str_replace(&html, "$locale", g_strdup (setlocale (LC_MESSAGES, NULL)), TRUE); webView = WEBKIT_WEB_VIEW(webkit_web_view_new());