321 lines
10 KiB
Diff
321 lines
10 KiB
Diff
From acd627a2fabe9856947399044dbf7aa79247c75b Mon Sep 17 00:00:00 2001
|
|
From: Ryan Gonzalez <ryan.gonzalez@collabora.com>
|
|
Date: Sat, 4 Mar 2023 16:23:37 -0600
|
|
Subject: [PATCH] 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>
|
|
---
|
|
app/flatpak-builtins-info.c | 8 +++-
|
|
app/flatpak-builtins-remote-info.c | 5 +-
|
|
app/flatpak-cli-transaction.c | 12 +++--
|
|
common/flatpak-utils-private.h | 11 +++++
|
|
common/flatpak-utils.c | 77 ++++++++++++++++++++++++++++++
|
|
tests/make-test-app.sh | 8 ++++
|
|
tests/test-info.sh | 14 ++++--
|
|
tests/testcommon.c | 39 +++++++++++++++
|
|
8 files changed, 164 insertions(+), 10 deletions(-)
|
|
|
|
diff --git a/app/flatpak-builtins-info.c b/app/flatpak-builtins-info.c
|
|
index c13d2d89eb..35d49c446a 100644
|
|
--- a/app/flatpak-builtins-info.c
|
|
+++ b/app/flatpak-builtins-info.c
|
|
@@ -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)
|
|
diff --git a/app/flatpak-builtins-remote-info.c b/app/flatpak-builtins-remote-info.c
|
|
index 20705a97ca..0ab05b7ca4 100644
|
|
--- a/app/flatpak-builtins-remote-info.c
|
|
+++ b/app/flatpak-builtins-remote-info.c
|
|
@@ -441,7 +441,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");
|
|
}
|
|
diff --git a/app/flatpak-cli-transaction.c b/app/flatpak-cli-transaction.c
|
|
index a258f905c4..b915bedd04 100644
|
|
--- a/app/flatpak-cli-transaction.c
|
|
+++ b/app/flatpak-cli-transaction.c
|
|
@@ -894,12 +894,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;
|
|
|
|
@@ -908,10 +912,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);
|
|
diff --git a/common/flatpak-utils-private.h b/common/flatpak-utils-private.h
|
|
index 1a92154706..c1282641f9 100644
|
|
--- a/common/flatpak-utils-private.h
|
|
+++ b/common/flatpak-utils-private.h
|
|
@@ -902,6 +902,17 @@ null_safe_g_ptr_array_unref (gpointer data)
|
|
g_clear_pointer (&data, g_ptr_array_unref);
|
|
}
|
|
|
|
+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);
|
|
+
|
|
#define FLATPAK_MESSAGE_ID "c7b39b1e006b464599465e105b361485"
|
|
|
|
#endif /* __FLATPAK_UTILS_H__ */
|
|
diff --git a/common/flatpak-utils.c b/common/flatpak-utils.c
|
|
index 3a5adb82ee..0b91ae3f91 100644
|
|
--- a/common/flatpak-utils.c
|
|
+++ b/common/flatpak-utils.c
|
|
@@ -8613,4 +8613,81 @@ flatpak_dconf_path_is_similar (const char *path1,
|
|
return (path1[i1] == '\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);
|
|
+}
|
|
diff --git a/tests/make-test-app.sh b/tests/make-test-app.sh
|
|
index 731160535c..125e97f6b5 100755
|
|
--- a/tests/make-test-app.sh
|
|
+++ b/tests/make-test-app.sh
|
|
@@ -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
|
|
diff --git a/tests/test-info.sh b/tests/test-info.sh
|
|
index 4a247bdb48..2158c11b10 100644
|
|
--- a/tests/test-info.sh
|
|
+++ b/tests/test-info.sh
|
|
@@ -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"
|
|
|
|
diff --git a/tests/testcommon.c b/tests/testcommon.c
|
|
index 1d217c072a..daae5e4aac 100644
|
|
--- a/tests/testcommon.c
|
|
+++ b/tests/testcommon.c
|
|
@@ -1557,6 +1557,44 @@ test_dconf_paths (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[])
|
|
{
|
|
@@ -1585,6 +1623,7 @@ main (int argc, char *argv[])
|
|
g_test_add_func ("/common/dconf-app-id", test_dconf_app_id);
|
|
g_test_add_func ("/common/dconf-paths", test_dconf_paths);
|
|
g_test_add_func ("/common/decompose-ref", test_decompose);
|
|
+ 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);
|