/pidgin/main: b2571530fa8b: Change how we handle clicking on fil...

Mark Doliner mark at kingant.net
Tue Jan 28 10:38:11 EST 2014

Changeset: b2571530fa8b7fd5d5e70c54412595e8e2f2fee2
Author:	 Mark Doliner <mark at kingant.net>
Date:	 2014-01-12 00:50 -0800
Branch:	 release-2.x.y
URL: https://hg.pidgin.im/pidgin/main/rev/b2571530fa8b


Change how we handle clicking on file:// links on Windows.

Previously we attempted to exec the file. This can be dangerous if
someone sends you a link to a malicious remote file. For now we're
going to open Explorer at the file's location. The user can decide
from there what they want to do--hopefully it'll be more obvious
what they're exec'ing and they can make a more informed decision.

This was a pretty easy change. We already had code to launch explorer.exe
that Eion wrote in this commit:

But due to a bug it was only getting triggered if the URI was

A possibly better approach is for us to show an "are you sure you
want to do this?" prompt. I don't want to do that in 2.x.y, but
we could do it in default.

We weren't correctly escaping the file path that we passed to explorer.exe.
I believe this would have allowed a remote users to craft links that pass
arbitrary parameters to explorer.exe. I think it is not possible to craft
links that would exec other commands, and the arguments to explorer.exe
look fairly innocuous, so I don't think this is a major problem. But of
course we should fix it--we want to dictate how file:// are opened, we
don't want remote users to be able to dictate this.

The old code called g_shell_quote() to attempt to escape the URI, but it
didn't actually use the return value. Additionally g_shell_quote()
doesn't do what we want. It wrapps the string in single quotes and
escapes single quotes with a backslash. We really just want to escape
double quotes with a double quote.

Incidentally, explorer.exe argument parsing is bat shit crazy [1]. Args
are separated by commas or equals (not spaces). Double-quotes can be
used to wrap an argument but somehow double-quotes within an argument are
ignored. If the first field in an argument is not '/' then the entire
thing is interpreted as a path (until the next comma or equals, I guess?)
For something that's been around for 20 years and is a core piece of
the OS you'd think it would have half-way respectable argument parsing.
Then again, it's Windows.

[1] http://www.geoffchappell.com/studies/windows/shell/explorer/cmdline.htm


 ChangeLog         |   3 ++
 pidgin/gtkutils.c |  59 ++++++++++++++++++++++++++----------------------------
 2 files changed, 31 insertions(+), 31 deletions(-)

diffs (135 lines):

diff --git a/ChangeLog b/ChangeLog
--- a/ChangeLog
+++ b/ChangeLog
@@ -16,6 +16,9 @@ version 2.10.8:
 	* Fix handling of multibyte UTF-8 characters in smiley themes. (#15756)
 	Windows-Specific Changes:
+	* When clicking file:// links, show the file in Explorer rather than
+	  attempting to run the file. This reduces the chances of a user
+	  clicking on a link and mistakenly running a malicious file.
 	* Fix Tcl scripts. (#15520)
 	* Fix crash-on-startup when ASLR is always on. (#15521)
 	* Updates to dependencies:
diff --git a/pidgin/gtkutils.c b/pidgin/gtkutils.c
--- a/pidgin/gtkutils.c
+++ b/pidgin/gtkutils.c
@@ -3267,33 +3267,28 @@ copy_email_address(GtkIMHtml *imhtml, Gt
 	return TRUE;
+ * @param filename The path to a file. Specifically this is the link target
+ *        from a link in an IM window with the leading "file://" removed.
+ */
 static void
-file_open_uri(GtkIMHtml *imhtml, const char *uri)
+open_file(GtkIMHtml *imhtml, const char *filename)
 	/* Copied from gtkft.c:open_button_cb */
 #ifdef _WIN32
 	/* If using Win32... */
 	int code;
-	if (purple_str_has_prefix(uri, "file://"))
-	{
-		gchar *escaped = g_shell_quote(uri);
-		gchar *param = g_strconcat("/select,\"", uri, "\"", NULL);
-		wchar_t *wc_param = g_utf8_to_utf16(param, -1, NULL, NULL, NULL);
-		code = (int)ShellExecuteW(NULL, L"OPEN", L"explorer.exe", wc_param, NULL, SW_NORMAL);
-		g_free(wc_param);
-		g_free(param);
-		g_free(escaped);
-	} else {
-		wchar_t *wc_filename = g_utf8_to_utf16(
-				uri, -1, NULL, NULL, NULL);
-		code = (int)ShellExecuteW(NULL, NULL, wc_filename, NULL, NULL,
-				SW_SHOW);
-		g_free(wc_filename);
-	}
+	/* Escape URI by replacing double-quote with 2 double-quotes. */
+	gchar *escaped = purple_strreplace(filename, "\"", "\"\"");
+	gchar *param = g_strconcat("/select,\"", escaped, "\"", NULL);
+	wchar_t *wc_param = g_utf8_to_utf16(param, -1, NULL, NULL, NULL);
+	/* TODO: Better to use SHOpenFolderAndSelectItems()? */
+	code = (int)ShellExecuteW(NULL, L"OPEN", L"explorer.exe", wc_param, NULL, SW_NORMAL);
+	g_free(wc_param);
+	g_free(param);
+	g_free(escaped);
@@ -3304,7 +3299,8 @@ file_open_uri(GtkIMHtml *imhtml, const c
 		purple_notify_error(imhtml, NULL,
 				_("An error occurred while opening the file."), NULL);
-		purple_debug_warning("gtkutils", "filename: %s; code: %d\n", uri, code);
+		purple_debug_warning("gtkutils", "filename: %s; code: %d\n",
+				filename, code);
 	char *command = NULL;
@@ -3313,15 +3309,15 @@ file_open_uri(GtkIMHtml *imhtml, const c
 	if (purple_running_gnome())
-		char *escaped = g_shell_quote(uri);
+		char *escaped = g_shell_quote(filename);
 		command = g_strdup_printf("gnome-open %s", escaped);
 	else if (purple_running_kde())
-		char *escaped = g_shell_quote(uri);
-		if (purple_str_has_suffix(uri, ".desktop"))
+		char *escaped = g_shell_quote(filename);
+		if (purple_str_has_suffix(filename, ".desktop"))
 			command = g_strdup_printf("kfmclient openURL %s 'text/plain'", escaped);
 			command = g_strdup_printf("kfmclient openURL %s", escaped);
@@ -3329,7 +3325,7 @@ file_open_uri(GtkIMHtml *imhtml, const c
-		purple_notify_uri(NULL, uri);
+		purple_notify_uri(NULL, filename);
@@ -3339,7 +3335,7 @@ file_open_uri(GtkIMHtml *imhtml, const c
 		if (!g_spawn_command_line_sync(command, NULL, NULL, &exit_status, &error))
 			tmp = g_strdup_printf(_("Error launching %s: %s"),
-							uri, error->message);
+							filename, error->message);
 			purple_notify_error(imhtml, NULL, _("Unable to open file."), tmp);
@@ -3360,8 +3356,9 @@ file_open_uri(GtkIMHtml *imhtml, const c
 static gboolean
 file_clicked_cb(GtkIMHtml *imhtml, GtkIMHtmlLink *link)
-	const char *uri = gtk_imhtml_link_get_url(link) + FILELINKSIZE;
-	file_open_uri(imhtml, uri);
+	/* Strip "file://" from the URI. */
+	const char *filename = gtk_imhtml_link_get_url(link) + FILELINKSIZE;
+	open_file(imhtml, filename);
 	return TRUE;
@@ -3369,7 +3366,7 @@ static gboolean
 open_containing_cb(GtkIMHtml *imhtml, const char *url)
 	char *dir = g_path_get_dirname(url + FILELINKSIZE);
-	file_open_uri(imhtml, dir);
+	open_file(imhtml, dir);
 	return TRUE;

More information about the Commits mailing list