/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
Description:
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:
https://hg.pidgin.im/pidgin/main/rev/4377067bda01
But due to a bug it was only getting triggered if the URI was
"file://file://something"
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.
REGARDING ESCAPING
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
diffstat:
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);
if (code == SE_ERR_ASSOCINCOMPLETE || code == SE_ERR_NOASSOC)
{
@@ -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);
}
#else
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);
g_free(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);
else
command = g_strdup_printf("kfmclient openURL %s", escaped);
@@ -3329,7 +3325,7 @@ file_open_uri(GtkIMHtml *imhtml, const c
}
else
{
- purple_notify_uri(NULL, uri);
+ purple_notify_uri(NULL, filename);
return;
}
@@ -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);
g_free(tmp);
g_error_free(error);
@@ -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);
g_free(dir);
return TRUE;
}
More information about the Commits
mailing list