Ensure special characters in permissions and metadata are escaped

This prevents someone from placing special characters in order to
manipulate the appearance of the permissions list.

CVE-2023-28101, GHSA-h43h-fwqx-mpp8

Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
This commit is contained in:
Ryan Gonzalez
2023-03-04 16:23:37 -06:00
committed by Simon McVittie
parent e67356a93e
commit 9e7ec07e67
8 changed files with 168 additions and 11 deletions

View File

@@ -400,7 +400,9 @@ flatpak_builtin_info (int argc, char **argv, GCancellable *cancellable, GError *
if (!g_file_load_contents (file, cancellable, &data, &data_size, NULL, error))
return FALSE;
g_print ("%s", data);
flatpak_print_escaped_string (data,
FLATPAK_ESCAPE_ALLOW_NEWLINES
| FLATPAK_ESCAPE_DO_NOT_QUOTE);
}
if (opt_show_permissions || opt_file_access)
@@ -421,7 +423,9 @@ flatpak_builtin_info (int argc, char **argv, GCancellable *cancellable, GError *
if (contents == NULL)
return FALSE;
g_print ("%s", contents);
flatpak_print_escaped_string (contents,
FLATPAK_ESCAPE_ALLOW_NEWLINES
| FLATPAK_ESCAPE_DO_NOT_QUOTE);
}
if (opt_file_access)

View File

@@ -431,7 +431,10 @@ flatpak_builtin_remote_info (int argc, char **argv, GCancellable *cancellable, G
if (opt_show_metadata)
{
g_print ("%s", xa_metadata ? xa_metadata : "");
if (xa_metadata != NULL)
flatpak_print_escaped_string (xa_metadata,
FLATPAK_ESCAPE_ALLOW_NEWLINES
| FLATPAK_ESCAPE_DO_NOT_QUOTE);
if (xa_metadata == NULL || !g_str_has_suffix (xa_metadata, "\n"))
g_print ("\n");
}

View File

@@ -1121,12 +1121,16 @@ print_perm_line (int idx,
int cols)
{
g_autoptr(GString) res = g_string_new (NULL);
g_autofree char *escaped_first_perm = NULL;
int i;
g_string_append_printf (res, " [%d] %s", idx, (char *) items->pdata[0]);
escaped_first_perm = flatpak_escape_string (items->pdata[0], FLATPAK_ESCAPE_DEFAULT);
g_string_append_printf (res, " [%d] %s", idx, escaped_first_perm);
for (i = 1; i < items->len; i++)
{
g_autofree char *escaped = flatpak_escape_string (items->pdata[i],
FLATPAK_ESCAPE_DEFAULT);
char *p;
int len;
@@ -1135,10 +1139,10 @@ print_perm_line (int idx,
p = res->str;
len = (res->str + strlen (res->str)) - p;
if (len + strlen ((char *) items->pdata[i]) + 2 >= cols)
g_string_append_printf (res, ",\n %s", (char *) items->pdata[i]);
if (len + strlen (escaped) + 2 >= cols)
g_string_append_printf (res, ",\n %s", escaped);
else
g_string_append_printf (res, ", %s", (char *) items->pdata[i]);
g_string_append_printf (res, ", %s", escaped);
}
g_print ("%s\n", res->str);

View File

@@ -928,6 +928,17 @@ gboolean flatpak_str_is_integer (const char *s);
gboolean flatpak_uri_equal (const char *uri1,
const char *uri2);
typedef enum {
FLATPAK_ESCAPE_DEFAULT = 0,
FLATPAK_ESCAPE_ALLOW_NEWLINES = 1 << 0,
FLATPAK_ESCAPE_DO_NOT_QUOTE = 1 << 1,
} FlatpakEscapeFlags;
char * flatpak_escape_string (const char *s,
FlatpakEscapeFlags flags);
void flatpak_print_escaped_string (const char *s,
FlatpakEscapeFlags flags);
gboolean running_under_sudo (void);
#define FLATPAK_MESSAGE_ID "c7b39b1e006b464599465e105b361485"

View File

@@ -625,7 +625,7 @@ load_kernel_module_list (void)
g_autofree char *modules_data = NULL;
g_autoptr(GError) error = NULL;
char *start, *end;
if (!g_file_get_contents ("/proc/modules", &modules_data, NULL, &error))
{
g_debug ("Failed to read /proc/modules: %s", error->message);
@@ -9221,6 +9221,86 @@ flatpak_uri_equal (const char *uri1,
return g_strcmp0 (uri1_norm, uri2_norm) == 0;
}
static gboolean
is_char_safe (gunichar c)
{
return g_unichar_isgraph (c) || c == ' ';
}
static gboolean
should_hex_escape (gunichar c,
FlatpakEscapeFlags flags)
{
if ((flags & FLATPAK_ESCAPE_ALLOW_NEWLINES) && c == '\n')
return FALSE;
return !is_char_safe (c);
}
static void
append_hex_escaped_character (GString *result,
gunichar c)
{
if (c <= 0xFF)
g_string_append_printf (result, "\\x%02X", c);
else if (c <= 0xFFFF)
g_string_append_printf (result, "\\u%04X", c);
else
g_string_append_printf (result, "\\U%08X", c);
}
char *
flatpak_escape_string (const char *s,
FlatpakEscapeFlags flags)
{
g_autoptr(GString) res = g_string_new ("");
gboolean did_escape = FALSE;
while (*s)
{
gunichar c = g_utf8_get_char_validated (s, -1);
if (c == (gunichar)-2 || c == (gunichar)-1)
{
/* Need to convert to unsigned first, to avoid negative chars becoming
huge gunichars. */
append_hex_escaped_character (res, (unsigned char)*s++);
did_escape = TRUE;
continue;
}
else if (should_hex_escape (c, flags))
{
append_hex_escaped_character (res, c);
did_escape = TRUE;
}
else if (c == '\\' || (!(flags & FLATPAK_ESCAPE_DO_NOT_QUOTE) && c == '\''))
{
g_string_append_printf (res, "\\%c", (char) c);
did_escape = TRUE;
}
else
g_string_append_unichar (res, c);
s = g_utf8_find_next_char (s, NULL);
}
if (did_escape && !(flags & FLATPAK_ESCAPE_DO_NOT_QUOTE))
{
g_string_prepend_c (res, '\'');
g_string_append_c (res, '\'');
}
return g_string_free (g_steal_pointer (&res), FALSE);
}
void
flatpak_print_escaped_string (const char *s,
FlatpakEscapeFlags flags)
{
g_autofree char *escaped = flatpak_escape_string (s, flags);
g_print ("%s", escaped);
}
gboolean
running_under_sudo (void)
{

View File

@@ -40,6 +40,14 @@ required-flatpak=$REQUIRED_VERSION
EOF
fi
if [ x${INCLUDE_SPECIAL_CHARACTER-} != x ]; then
TAB=$'\t'
cat >> ${DIR}/metadata <<EOF
[Environment]
A=x${TAB}y
EOF
fi
cat >> ${DIR}/metadata <<EOF
[Extension $APP_ID.Locale]
directory=share/runtime/locale

View File

@@ -6,9 +6,9 @@ set -euo pipefail
skip_revokefs_without_fuse
echo "1..7"
echo "1..8"
setup_repo
INCLUDE_SPECIAL_CHARACTER=1 setup_repo
install_repo
COMMIT=`${FLATPAK} ${U} info --show-commit org.test.Hello`
@@ -19,9 +19,17 @@ assert_file_has_content info "^app/org\.test\.Hello/$(flatpak --default-arch)/ma
ok "info -rcos"
${FLATPAK} info --show-metadata org.test.Hello > info
# CVE-2023-28101
assert_file_has_content info "name=org\.test\.Hello"
assert_file_has_content info "^A=x\\\\x09y"
ok "info --show-metadata"
${FLATPAK} info --show-permissions org.test.Hello > info
assert_file_empty info
assert_file_has_content info "^A=x\\\\x09y"
ok "info --show-permissions"

View File

@@ -1837,6 +1837,44 @@ test_parse_x11_display (void)
}
}
typedef struct {
const char *in;
FlatpakEscapeFlags flags;
const char *out;
} EscapeData;
static EscapeData escapes[] = {
{"abc def", FLATPAK_ESCAPE_DEFAULT, "abc def"},
{"やあ", FLATPAK_ESCAPE_DEFAULT, "やあ"},
{"\033[;1m", FLATPAK_ESCAPE_DEFAULT, "'\\x1B[;1m'"},
// non-printable U+061C
{"\u061C", FLATPAK_ESCAPE_DEFAULT, "'\\u061C'"},
// non-printable U+1343F
{"\xF0\x93\x90\xBF", FLATPAK_ESCAPE_DEFAULT, "'\\U0001343F'"},
// invalid utf-8
{"\xD8\1", FLATPAK_ESCAPE_DEFAULT, "'\\xD8\\x01'"},
{"\b \n abc ' \\", FLATPAK_ESCAPE_DEFAULT, "'\\x08 \\x0A abc \\' \\\\'"},
{"\b \n abc ' \\", FLATPAK_ESCAPE_DO_NOT_QUOTE, "\\x08 \\x0A abc ' \\\\"},
{"abc\tdef\n\033[;1m ghi\b", FLATPAK_ESCAPE_ALLOW_NEWLINES | FLATPAK_ESCAPE_DO_NOT_QUOTE,
"abc\\x09def\n\\x1B[;1m ghi\\x08"},
};
/* CVE-2023-28101 */
static void
test_string_escape (void)
{
gsize idx;
for (idx = 0; idx < G_N_ELEMENTS (escapes); idx++)
{
EscapeData *data = &escapes[idx];
g_autofree char *ret = NULL;
ret = flatpak_escape_string (data->in, data->flags);
g_assert_cmpstr (ret, ==, data->out);
}
}
int
main (int argc, char *argv[])
{
@@ -1870,6 +1908,7 @@ main (int argc, char *argv[])
g_test_add_func ("/common/quote-argv", test_quote_argv);
g_test_add_func ("/common/str-is-integer", test_str_is_integer);
g_test_add_func ("/common/parse-x11-display", test_parse_x11_display);
g_test_add_func ("/common/string-escape", test_string_escape);
g_test_add_func ("/app/looks-like-branch", test_looks_like_branch);
g_test_add_func ("/app/columns", test_columns);