Disallow @@ and @@u usage in desktop files #4148
Conversation
|
Can one of the admins verify this patch?
|
There was a problem hiding this comment.
Maintainers: please could someone look at this as a matter of some urgency? I think this should be handled as a security issue and fixed in stable branches and distro packages.
@refi64's proposed change looks good to me, with or without the changes I suggest. I'm going to apply it in Debian now.
It's unfortunate that this was reported as a public issue. It would be great if Flatpak had a documented path for reporting security issues, similar to bubblewrap's.
| @@ -7139,6 +7139,8 @@ export_desktop_file (const char *app, | |||
| g_string_append_printf (new_exec, " @@ %s @@", arg); | |||
| else if (strcasecmp (arg, "%u") == 0) | |||
| g_string_append_printf (new_exec, " @@u %s @@", arg); | |||
| else if (strcmp (arg, "@@") == 0 || strcmp (arg, "@@u") == 0) | |||
There was a problem hiding this comment.
Since we already have two special tokens, @@ and @@u, I'd be tempted to assume we will eventually need to invent a third, and assume that it will be @@x or something:
| else if (strcmp (arg, "@@") == 0 || strcmp (arg, "@@u") == 0) | |
| else if (g_str_has_prefix (arg, "@@")) |
There was a problem hiding this comment.
yeah, seems best to just reserve the entire @@ prefix
There was a problem hiding this comment.
#4156 incorporates this suggestion.
| @@ -7139,6 +7139,8 @@ export_desktop_file (const char *app, | |||
| g_string_append_printf (new_exec, " @@ %s @@", arg); | |||
| else if (strcasecmp (arg, "%u") == 0) | |||
| g_string_append_printf (new_exec, " @@u %s @@", arg); | |||
| else if (strcmp (arg, "@@") == 0 || strcmp (arg, "@@u") == 0) | |||
| g_print (_("Skipping invalid Exec argument %s\n"), arg); | |||
There was a problem hiding this comment.
Since this is either an attempted attack or something that will break %u and %f processing, I'd be tempted to make it fail hard:
| g_print (_("Skipping invalid Exec argument %s\n"), arg); | |
| { | |
| flatpak_fail_error (error, FLATPAK_ERROR_EXPORT_FAILED, | |
| _("Invalid Exec argument %s"), arg); | |
| goto out; | |
| } |
There was a problem hiding this comment.
#4156 incorporates this suggestion.
|
I've opened #4155 to try to document the security policy as I understand it, but it's missing a private contact route that reporters of security vulnerabilities can use. |
|
closed in favour of #4156 |
Fixes #4146.