GMenu does not handle desktop file exec strings properly

[Tristan Moody]

Using GMenu plugin on cairo-dock-2.1.2.4 on Fedora 12:

Applications in the main menu whose .desktop file exec string contains quotes (single or double) don't get launched.

Example: Launching the application Kst from GMenu
exec=kst -caption "%c" %i %m

cairo-dock -l debug output:
debug : (cairo-dock-callbacks.c:cairo_dock_launch_command_full:955)
  cairo_dock_launch_command_full (kst -caption " , (null))
sh: -c: line 0: unexpected EOF while looking for matching `"'
sh: -c: line 1: syntax error: unexpected end of file

by the time the exec string gets to cairo_dock_launch_command_full, any text appearing after quotes is truncated, causing the program to fail to launch.

workaround: modify .desktop files to remove quotes from exec strings

suggestion: GMenu/applet-utils.c:89 --> change g_key_file_get_string to g_key_file_get_value

Thoughts? Does this open up any security issues?

Reasoning:
g_key_file_parse_value_as_string, as called by g_key_file_get_string takes the result of g_key_file_get_value and processes some escape sequences. However, it does not, for some reason, handle escapes such as \' and \", which mangles the exec string, causing the program not to launch.

[Fabounet (fabounet03)]

Hi,
thanks for the information.
Do you know what %c, %i and %m are for ?
When we click on the menu entry, shouldn't we expect the program to just
launch itself, without taking any argument ?

2010/2/23 Tristan Moody <email address hidden>

> Public bug reported:
>
> Using GMenu plugin on cairo-dock-2.1.2.4 on Fedora 12:
>
> Applications in the main menu whose .desktop file exec string contains
> quotes (single or double) don't get launched.
>
> Example: Launching the application Kst from GMenu
> exec=kst -caption "%c" %i %m
>
> cairo-dock -l debug output:
> debug : (cairo-dock-callbacks.c:cairo_dock_launch_command_full:955)
> cairo_dock_launch_command_full (kst -caption " , (null))
> sh: -c: line 0: unexpected EOF while looking for matching `"'
> sh: -c: line 1: syntax error: unexpected end of file
>
> by the time the exec string gets to cairo_dock_launch_command_full, any
> text appearing after quotes is truncated, causing the program to fail to
> launch.
>
> workaround: modify .desktop files to remove quotes from exec strings
>
> suggestion: GMenu/applet-utils.c:89 --> change g_key_file_get_string to
> g_key_file_get_value
>
> Thoughts? Does this open up any security issues?
>
> Reasoning:
> g_key_file_parse_value_as_string, as called by g_key_file_get_string takes
> the result of g_key_file_get_value and processes some escape sequences.
> However, it does not, for some reason, handle escapes such as \' and \",
> which mangles the exec string, causing the program not to launch.
>
> ** Affects: cairo-dock-core
> Importance: Undecided
> Status: New
>
> --
> GMenu does not handle desktop file exec strings properly
> https://bugs.launchpad.net/bugs/526138
> You received this bug notification because you are a member of Cairo-
> Dock Team, which is subscribed to Cairo-Dock Core.
>
> Status in Cairo-Dock : Core: New
>
> Bug description:
> Using GMenu plugin on cairo-dock-2.1.2.4 on Fedora 12:
>
> Applications in the main menu whose .desktop file exec string contains
> quotes (single or double) don't get launched.
>
> Example: Launching the application Kst from GMenu
> exec=kst -caption "%c" %i %m
>
> cairo-dock -l debug output:
> debug : (cairo-dock-callbacks.c:cairo_dock_launch_command_full:955)
> cairo_dock_launch_command_full (kst -caption " , (null))
> sh: -c: line 0: unexpected EOF while looking for matching `"'
> sh: -c: line 1: syntax error: unexpected end of file
>
> by the time the exec string gets to cairo_dock_launch_command_full, any
> text appearing after quotes is truncated, causing the program to fail to
> launch.
>
> workaround: modify .desktop files to remove quotes from exec strings
>
> suggestion: GMenu/applet-utils.c:89 --> change g_key_file_get_string to
> g_key_file_get_value
>
> Thoughts? Does this open up any security issues?
>
> Reasoning:
> g_key_file_parse_value_as_string, as called by g_key_file_get_string takes
> the result of g_key_file_get_value and processes some escape sequences.
> However, it does not, for some reason, handle escapes such as \' and \",
> which mangles the exec string, causing the program not to launch.
>
>
>
> _______________________________________________
> Mailing list: https:...

[Tristan Moody (tmoody)]

The arguments have to do with the window decorator and are found in most KDE launchers.

Per http://standards.freedesktop.org/desktop-entry-spec/latest/ ...
%c expands to the Name key appropriate to the localization. Starting an app with the argument -caption "<thecaption>" tells the window decorator to replace the default window title with <thecaption>.

%i expands to the icon key, and %m is deprecated, but used to refer to the miniicon key.

Now, as to why g_key_file_get_string doesn't handle quotes properly, I really don't know why. My guess is it is a bug in that library, as I can't think of a reason for that to be deliberate, but running through a gdb session using ..._get_value instead seems to solve the problem.

[Tristan Moody (tmoody)]

Actually....

looking further into the Desktop file spec: http://standards.freedesktop.org/desktop-entry-spec/latest/ar01s06.html

>>>Quoting must be done by enclosing the argument between double quotes and escaping the double quote character, backtick character ("`"), dollar sign ("$") and backslash character ("\") by preceding it with an additional backslash character. Implementations must undo quoting before expanding field codes and before passing the argument to the executable program. Reserved characters are space (" "), tab, newline, double quote, single quote ("'"), backslash character ("\"), greater-than sign (">"), less-than sign ("<"), tilde ("~"), vertical bar ("|"), ampersand ("&"), semicolon (";"), dollar sign ("$"), asterisk ("*"), question mark ("?"), hash mark ("#"), parenthesis ("(") and (")") and backtick character ("`"). <<<

-----------------------

Double checking, when quotes are replaced by escaped quotes, the application opens normally. So technically, I suppose GMenu does in fact follow the spec properly. Interestingly, the menu associated with gnome-panel does not care whether quotes are properly escaped, so strictly speaking, the bug is in the gnome-panel menu and in the installation files for the applications that use the quotes improperly. However, it does not get noticed except when a program actually adheres to the spec, so the "bug" here is actually a strict adherence to the freedesktop spec.

I will look further into gnome-panel and file a report with gnome if necessary.

[Fabounet (fabounet03)]

ok thanks for you help !
well if g_key_file_get_string doesn't do its job but g_key_file_get_value does, I see no objection to use it.
So if you confirm that everything works fine this way, I'll make the replacement.

[Tristan Moody (tmoody)]

Ok, so using a rebuilt version that uses ..._get_value:

cCommand now fills with the entire command value, BUT...
in _launch_from_file:
107 if (cCommand != NULL)
108 {
109 gchar *str = strchr (cCommand, '%');
110 if (str != NULL)
111 *str = '\0';

This sequence truncates the string "kst -caption \"%c\" %i %m" to just "kst -caption \"".
Since there is no code to handle the freedesktop field codes, the function cairo_dock_launch_command_full() is fed an invalid command that chokes with a "no EOF" error when fed to system(). So, long story short, using g_key_file_get_value is a step in the right direction, but we're not there yet.

I did compare it to the gnome-panel code that performs the equivalent task, and gnome-panel handles the launchers in a completely different way. I think I can come up with a patch for this function that will get the right functionality in there without too big a change--only adding a few lines to applet-util.c to handle the various freedesktop field codes.

I'll let you know what I come up with.

[Tristan Moody (tmoody)]

Ok, I modified applet-util.c to parse the field codes and am attaching it to this post. It seems to work for me. Let me know what you think.

Tristan

[Fabounet (fabounet03)]

Thanks a lot, the patch is applied now.