From 04879fdea59bc3d30d9e700923720bfb34673e86 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 3 Sep 2015 12:44:33 +0200 Subject: Store and verify parent dir dev/ino and pass O_PATH fds In order to be robust against symlink attacks (i.e. make a document for a path, then replace it with a symlink somewhere else and have the portal read that instead) we store the parent dev/ino when we create the document id and always verify that (atomically with the *at syscalls) on each use. Also, we pass O_PATH fds when creating documents, as it allows us to be a bit safer. For instance we can verify that the fd is a O_PATH fd before doing any ops on it, and it makes it possible to avoid other symlink trickery. Also, we drop the double add methods, and just use the O_PATH version. --- document-portal/xdp-fuse.c | 224 ++++++++++++++++++++++++++++----------------- document-portal/xdp-main.c | 93 ++++++++----------- document-portal/xdp-util.c | 82 +++++++++++++++-- document-portal/xdp-util.h | 29 ++++-- 4 files changed, 272 insertions(+), 156 deletions(-) (limited to 'document-portal') diff --git a/document-portal/xdp-fuse.c b/document-portal/xdp-fuse.c index cbaaa6e..22e70ae 100644 --- a/document-portal/xdp-fuse.c +++ b/document-portal/xdp-fuse.c @@ -108,7 +108,7 @@ get_entry_cache_time (int st_mode) * the list (add/remove) or name of a tmpfile. * * Each instance has a mutex that locks access to the backing path, - * as it can be removed at runtime. Use get/steal_backing_path() to + * as it can be removed at runtime. Use get/steal_backing_basename() to * safely access it. * ******************************* XdpTmp *******************************/ @@ -122,6 +122,7 @@ typedef struct /* These are immutable, no lock needed */ guint64 parent_inode; guint32 tmp_id; + XdgAppDbEntry *entry; /* Changes always done under tmp_files lock */ char *name; @@ -129,7 +130,7 @@ typedef struct GMutex mutex; /* protected by mutex */ - char *backing_path; + char *backing_basename; } XdpTmp; /* Owns a ref to the files */ @@ -148,30 +149,31 @@ xdp_tmp_unref (XdpTmp *tmp) { if (g_atomic_int_dec_and_test (&tmp->ref_count)) { + xdg_app_db_entry_unref (tmp->entry); g_free (tmp->name); - g_free (tmp->backing_path); + g_free (tmp->backing_basename); g_free (tmp); } } char * -xdp_tmp_get_backing_path (XdpTmp *tmp) +xdp_tmp_get_backing_basename (XdpTmp *tmp) { char *res; g_mutex_lock (&tmp->mutex); - res = g_strdup (tmp->backing_path); + res = g_strdup (tmp->backing_basename); g_mutex_unlock (&tmp->mutex); return res; } char * -xdp_tmp_steal_backing_path (XdpTmp *tmp) +xdp_tmp_steal_backing_basename (XdpTmp *tmp) { char *res; g_mutex_lock (&tmp->mutex); - res = tmp->backing_path; - tmp->backing_path = NULL; + res = tmp->backing_basename; + tmp->backing_basename = NULL; g_mutex_unlock (&tmp->mutex); return res; @@ -227,17 +229,25 @@ find_tmp_by_id (guint32 tmp_id) /* Caller must hold tmp_files lock */ static XdpTmp * xdp_tmp_new_nolock (fuse_ino_t parent, + XdgAppDbEntry *entry, const char *name, - const char *path) + const char *tmp_basename) { XdpTmp *tmp; + g_autofree char *tmp_dirname = NULL; + + /* We store the pathname instead of dir_fd + basename, because + its very easy to get a lot of tempfiles leaking and that would + mean quite a lot of open fds */ + tmp_dirname = xdp_entry_dup_dirname (entry); tmp = g_new0 (XdpTmp, 1); + tmp->ref_count = 2; /* One owned by tmp_files */ + tmp->tmp_id = g_atomic_int_add (&next_tmp_id, 1); tmp->parent_inode = parent; tmp->name = g_strdup (name); - tmp->backing_path = g_strdup (path); - tmp->tmp_id = g_atomic_int_add (&next_tmp_id, 1); - tmp->ref_count = 1; /* Owned by tmp_files */ + tmp->entry = xdg_app_db_entry_ref (entry); + tmp->backing_basename = g_strdup (tmp_basename); tmp_files = g_list_prepend (tmp_files, tmp); @@ -249,11 +259,15 @@ static void xdp_tmp_unlink_nolock (XdpTmp *tmp) { - g_autofree char *backing_path = NULL; + g_autofree char *backing_basename = NULL; - backing_path = xdp_tmp_steal_backing_path (tmp); - if (backing_path) - unlink (backing_path); + backing_basename = xdp_tmp_steal_backing_basename (tmp); + if (backing_basename) + { + glnx_fd_close int dir_fd = xdp_entry_open_dir (tmp->entry); + if (dir_fd) + unlinkat (dir_fd, backing_basename, 0); + } tmp_files = g_list_remove (tmp_files, tmp); xdp_tmp_unref (tmp); @@ -291,8 +305,9 @@ typedef struct /* These are immutable, no lock needed */ guint32 tmp_id; fuse_ino_t inode; - char *trunc_path; - char *real_path; + int dir_fd; + char *trunc_basename; + char *real_basename; /* These need a lock whenever they are used */ int fd; @@ -319,24 +334,24 @@ xdp_fh_finalize (XdpFh *fh) if (fh->truncated) { fsync (fh->trunc_fd); - if (rename (fh->trunc_path, fh->real_path) != 0) + if (renameat (fh->dir_fd, fh->trunc_basename, + fh->dir_fd, fh->real_basename) != 0) g_warning ("Unable to replace truncated document"); } - else if (fh->trunc_path) - unlink (fh->trunc_path); + else if (fh->trunc_basename) + unlinkat (fh->dir_fd, fh->trunc_basename, 0); if (fh->fd >= 0) close (fh->fd); - if (fh->trunc_fd >= 0) - close (fh->trunc_fd); - if (fh->fd >= 0) - close (fh->fd); if (fh->trunc_fd >= 0) close (fh->trunc_fd); - g_clear_pointer (&fh->trunc_path, g_free); - g_clear_pointer (&fh->real_path, g_free); + if (fh->dir_fd >= 0) + close (fh->dir_fd); + + g_clear_pointer (&fh->trunc_basename, g_free); + g_clear_pointer (&fh->real_basename, g_free); g_free (fh); } @@ -398,6 +413,7 @@ xdp_fh_new (fuse_ino_t inode, fh->fd = fd; if (tmp) fh->tmp_id = tmp->tmp_id; + fh->dir_fd = -1; fh->trunc_fd = -1; fh->ref_count = 1; /* Owned by fuse_file_info fi */ @@ -656,7 +672,7 @@ app_can_see_doc (XdgAppDbEntry *entry, guint32 app_id) const char *app_name = get_app_name_from_id (app_id); if (app_name != NULL && - xdp_has_permissions (entry, app_name, XDP_PERMISSION_FLAGS_READ)) + xdp_entry_has_permissions (entry, app_name, XDP_PERMISSION_FLAGS_READ)) return TRUE; return FALSE; @@ -670,10 +686,9 @@ xdp_stat (fuse_ino_t ino, XdpInodeClass class = get_class (ino); guint64 class_ino = get_class_ino (ino); g_autoptr (XdgAppDbEntry) entry = NULL; - const char *path = NULL; struct stat tmp_stbuf; g_autoptr(XdpTmp) tmp = NULL; - g_autofree char *backing_path = NULL; + g_autofree char *backing_basename = NULL; stbuf->st_ino = ino; @@ -736,8 +751,7 @@ xdp_stat (fuse_ino_t ino, stbuf->st_nlink = DOC_FILE_NLINK; - path = xdp_get_path (entry); - if (stat (path, &tmp_stbuf) != 0) + if (xdp_entry_stat (entry, &tmp_stbuf, AT_SYMLINK_NOFOLLOW) != 0) return ENOENT; stbuf->st_mode = S_IFREG | get_user_perms (&tmp_stbuf); @@ -759,10 +773,16 @@ xdp_stat (fuse_ino_t ino, stbuf->st_mode = S_IFREG; stbuf->st_nlink = DOC_FILE_NLINK; - backing_path = xdp_tmp_get_backing_path (tmp); + backing_basename = xdp_tmp_get_backing_basename (tmp); - if (backing_path == NULL || stat (backing_path, &tmp_stbuf) != 0) - return ENOENT; + { + glnx_fd_close int dir_fd = xdp_entry_open_dir (tmp->entry); + + if (backing_basename == NULL || + dir_fd == -1 || + fstatat (dir_fd, backing_basename, &tmp_stbuf, 0) != 0) + return ENOENT; + } stbuf->st_mode = S_IFREG | get_user_perms (&tmp_stbuf); stbuf->st_size = tmp_stbuf.st_size; @@ -905,7 +925,7 @@ xdp_lookup (fuse_ino_t parent, entry = xdp_lookup_doc (parent_class_ino); if (entry != NULL) { - g_autofree char *basename = xdp_dup_basename (entry); + g_autofree char *basename = xdp_entry_dup_basename (entry); if (strcmp (name, basename) == 0) { *inode = make_inode (DOC_FILE_INO_CLASS, parent_class_ino); @@ -1036,9 +1056,8 @@ dirbuf_add_doc_file (fuse_req_t req, guint32 doc_id) { struct stat tmp_stbuf; - const char *path = xdp_get_path (entry); - g_autofree char *basename = xdp_dup_basename (entry); - if (stat (path, &tmp_stbuf) == 0) + g_autofree char *basename = xdp_entry_dup_basename (entry); + if (xdp_entry_stat (entry, &tmp_stbuf, AT_SYMLINK_NOFOLLOW) == 0) dirbuf_add (req, b, basename, make_inode (DOC_FILE_INO_CLASS, doc_id)); } @@ -1222,14 +1241,13 @@ get_open_flags (struct fuse_file_info *fi) } static char * -create_tmp_for_doc (XdgAppDbEntry *entry, int flags, int *fd_out) +create_tmp_for_doc (XdgAppDbEntry *entry, int dir_fd, int flags, int *fd_out) { - g_autofree char *dirname = xdp_dup_dirname (entry); - g_autofree char *basename = xdp_dup_basename (entry); - g_autofree char *template = g_strconcat (dirname, "/.", basename, ".XXXXXX", NULL); + g_autofree char *basename = xdp_entry_dup_basename (entry); + g_autofree char *template = g_strconcat (".xdp_", basename, ".XXXXXX", NULL); int fd; - fd = g_mkstemp_full (template, flags, 0600); + fd = xdg_app_mkstempat (dir_fd, template, flags, 0600); if (fd == -1) return NULL; @@ -1246,7 +1264,6 @@ xdp_fuse_open (fuse_req_t req, guint64 class_ino = get_class_ino (ino); struct stat stbuf = {0}; g_autoptr (XdgAppDbEntry) entry = NULL; - const char *path = NULL; g_autoptr(XdpTmp) tmp = NULL; glnx_fd_close int fd = -1; int res; @@ -1268,50 +1285,59 @@ xdp_fuse_open (fuse_req_t req, if (entry && class == DOC_FILE_INO_CLASS) { - g_autofree char *write_path = NULL; + g_autofree char *tmp_basename = NULL; glnx_fd_close int write_fd = -1; + glnx_fd_close int dir_fd = -1; + g_autofree char *basename = xdp_entry_dup_basename (entry); - path = xdp_get_path (entry); + dir_fd = xdp_entry_open_dir (entry); + if (dir_fd == -1) + { + fuse_reply_err (req, errno); + return; + } if ((fi->flags & 3) != O_RDONLY) { - if (access (path, W_OK) != 0) + if (faccessat (dir_fd, basename, W_OK, 0) != 0) { fuse_reply_err (req, errno); return; } - write_path = create_tmp_for_doc (entry, O_RDWR, &write_fd); - if (write_path == NULL) + tmp_basename = create_tmp_for_doc (entry, dir_fd, O_RDWR, &write_fd); + if (tmp_basename == NULL) { fuse_reply_err (req, errno); return; } } - fd = open (path, O_RDONLY); + fd = openat (dir_fd, basename, O_RDONLY|O_NOFOLLOW|O_CLOEXEC); if (fd < 0) { fuse_reply_err (req, errno); return; } fh = xdp_fh_new (ino, fi, steal_fd(&fd), NULL); + fh->dir_fd = steal_fd (&dir_fd); fh->trunc_fd = steal_fd (&write_fd); - fh->trunc_path = g_steal_pointer (&write_path); - fh->real_path = g_strdup (path); + fh->trunc_basename = g_steal_pointer (&tmp_basename); + fh->real_basename = g_strdup (basename); if (fuse_reply_open (req, fi)) xdp_fh_unref (fh); } else if (class == TMPFILE_INO_CLASS && (tmp = find_tmp_by_id (class_ino))) { - g_autofree char *backing_path = xdp_tmp_get_backing_path (tmp); - if (backing_path == NULL) + glnx_fd_close int dir_fd = xdp_entry_open_dir (tmp->entry); + g_autofree char *backing_basename = xdp_tmp_get_backing_basename (tmp); + if (dir_fd == -1 || backing_basename == NULL) { fuse_reply_err (req, ENOENT); return; } - fd = open (backing_path, get_open_flags (fi)); + fd = openat (dir_fd, backing_basename, get_open_flags (fi)); if (fd < 0) { fuse_reply_err (req, errno); @@ -1338,7 +1364,6 @@ xdp_fuse_create (fuse_req_t req, XdpFh *fh; g_autoptr(XdgAppDbEntry) entry = NULL; g_autofree char *basename = NULL; - const char *path = NULL; glnx_fd_close int fd = -1; int res; @@ -1363,23 +1388,29 @@ xdp_fuse_create (fuse_req_t req, return; } - basename = xdp_dup_basename (entry); + basename = xdp_entry_dup_basename (entry); if (strcmp (name, basename) == 0) { - g_autofree char *write_path = NULL; + g_autofree char *tmp_basename = NULL; glnx_fd_close int write_fd = -1; guint32 doc_id = xdp_id_from_name (name); + glnx_fd_close int dir_fd = -1; - write_path = create_tmp_for_doc (entry, O_RDWR, &write_fd); - if (write_path == NULL) + dir_fd = xdp_entry_open_dir (entry); + if (dir_fd == -1) { fuse_reply_err (req, errno); return; } - path = xdp_get_path (entry); + tmp_basename = create_tmp_for_doc (entry, dir_fd, O_RDWR, &write_fd); + if (tmp_basename == NULL) + { + fuse_reply_err (req, errno); + return; + } - fd = open (path, O_CREAT|O_EXCL|O_RDONLY); + fd = openat (dir_fd, basename, O_CREAT|O_EXCL|O_RDONLY|O_NOFOLLOW|O_CLOEXEC); if (fd < 0) { fuse_reply_err (req, errno); @@ -1389,10 +1420,11 @@ xdp_fuse_create (fuse_req_t req, e.ino = make_inode (DOC_FILE_INO_CLASS, doc_id); fh = xdp_fh_new (e.ino, fi, steal_fd (&fd), NULL); + fh->dir_fd = steal_fd (&dir_fd); fh->truncated = TRUE; fh->trunc_fd = steal_fd (&write_fd); - fh->trunc_path = g_steal_pointer (&write_path); - fh->real_path = g_strdup (path); + fh->trunc_basename = g_steal_pointer (&tmp_basename); + fh->real_basename = g_strdup (basename); if (xdp_fh_fstat_locked (fh, &e.attr) != 0) { @@ -1422,18 +1454,19 @@ xdp_fuse_create (fuse_req_t req, if (tmpfile) { - g_autofree char *backing_path = NULL; + glnx_fd_close int dir_fd = xdp_entry_open_dir (tmpfile->entry); + g_autofree char *backing_basename = NULL; G_UNLOCK(tmp_files); - backing_path = xdp_tmp_get_backing_path (tmpfile); - if (backing_path == NULL) + backing_basename = xdp_tmp_get_backing_basename (tmpfile); + if (dir_fd == -1 || backing_basename == NULL) { fuse_reply_err (req, EINVAL); return; } - fd = open (backing_path, get_open_flags (fi)); + fd = openat (dir_fd, backing_basename, get_open_flags (fi)); if (fd == -1) { fuse_reply_err (req, errno); @@ -1443,13 +1476,21 @@ xdp_fuse_create (fuse_req_t req, else { int errsv; - g_autofree char *tmp_path = NULL; + g_autofree char *tmp_basename = NULL; + glnx_fd_close int dir_fd = -1; + + dir_fd = xdp_entry_open_dir (entry); + if (dir_fd == -1) + { + fuse_reply_err (req, errno); + return; + } - tmp_path = create_tmp_for_doc (entry, get_open_flags (fi), &fd); - if (tmp_path == NULL) - return NULL; + tmp_basename = create_tmp_for_doc (entry, dir_fd, get_open_flags (fi), &fd); + if (tmp_basename == NULL) + return; - tmpfile = xdp_tmp_new_nolock (parent, name, tmp_path); + tmpfile = xdp_tmp_new_nolock (parent, entry, name, tmp_basename); errsv = errno; G_UNLOCK(tmp_files); @@ -1626,17 +1667,24 @@ xdp_fuse_rename (fuse_req_t req, return; } - basename = xdp_dup_basename (entry); + basename = xdp_entry_dup_basename (entry); if (strcmp (newname, basename) == 0) { - const char *real_path = xdp_get_path (entry); - g_autofree char *backing_path = NULL; + glnx_fd_close int dir_fd = -1; + g_autofree char *backing_basename = NULL; /* Rename tmpfile to regular file */ + dir_fd = xdp_entry_open_dir (entry); + if (dir_fd == -1) + { + fuse_reply_err (req, errno); + return; + } + /* Steal backing path so we don't delete it when unlinking tmp */ - backing_path = xdp_tmp_steal_backing_path (tmp); - if (backing_path == NULL) + backing_basename = xdp_tmp_steal_backing_basename (tmp); + if (backing_basename == NULL) { fuse_reply_err (req, EINVAL); return; @@ -1645,7 +1693,8 @@ xdp_fuse_rename (fuse_req_t req, /* Stop writes to all outstanding fds to the temp file */ mark_open_tmp_file_readonly (tmp->tmp_id); - if (rename (backing_path, real_path) != 0) + if (renameat (dir_fd, backing_basename, + dir_fd, basename) != 0) { fuse_reply_err (req, errno); return; @@ -1797,7 +1846,7 @@ xdp_fuse_fsyncdir (fuse_req_t req, entry = xdp_lookup_doc (doc_id); if (entry != NULL) { - g_autofree char *dirname = xdp_dup_dirname (entry); + g_autofree char *dirname = xdp_entry_dup_dirname (entry); int fd = open (dirname, O_DIRECTORY|O_RDONLY); if (fd >= 0) { @@ -1868,12 +1917,19 @@ xdp_fuse_unlink (fuse_req_t req, return; } - basename = xdp_dup_basename (entry); + basename = xdp_entry_dup_basename (entry); if (strcmp (name, basename) == 0) { - const char *real_path = xdp_get_path (entry); + glnx_fd_close int dir_fd = -1; + + dir_fd = xdp_entry_open_dir (entry); + if (dir_fd == -1) + { + fuse_reply_err (req, errno); + return; + } - if (unlink (real_path) != 0) + if (unlinkat (dir_fd, basename, 0) != 0) { fuse_reply_err (req, errno); return; diff --git a/document-portal/xdp-main.c b/document-portal/xdp-main.c index f6f9d7e..cace699 100644 --- a/document-portal/xdp-main.c +++ b/document-portal/xdp-main.c @@ -134,7 +134,8 @@ portal_grant_permissions (GDBusMethodInvocation *invocation, perms = xdp_parse_permissions (permissions); /* Must have grant-permissions and all the newly granted permissions */ - if (!xdp_has_permissions (entry, app_id, XDP_PERMISSION_FLAGS_GRANT_PERMISSIONS | perms)) + if (!xdp_entry_has_permissions (entry, app_id, + XDP_PERMISSION_FLAGS_GRANT_PERMISSIONS | perms)) { g_dbus_method_invocation_return_error (invocation, XDG_APP_ERROR, XDG_APP_ERROR_NOT_ALLOWED, "Not enough permissions"); @@ -142,7 +143,7 @@ portal_grant_permissions (GDBusMethodInvocation *invocation, } do_set_permissions (entry, id, target_app_id, - perms | xdp_get_permissions (entry, target_app_id)); + perms | xdp_entry_get_permissions (entry, target_app_id)); g_dbus_method_invocation_return_value (invocation, g_variant_new ("()")); } @@ -178,7 +179,8 @@ portal_revoke_permissions (GDBusMethodInvocation *invocation, perms = xdp_parse_permissions (permissions); /* Must have grant-permissions, or be itself */ - if (!xdp_has_permissions (entry, app_id, XDP_PERMISSION_FLAGS_GRANT_PERMISSIONS) || + if (!xdp_entry_has_permissions (entry, app_id, + XDP_PERMISSION_FLAGS_GRANT_PERMISSIONS) || strcmp (app_id, target_app_id) == 0) { g_dbus_method_invocation_return_error (invocation, XDG_APP_ERROR, XDG_APP_ERROR_NOT_ALLOWED, @@ -187,7 +189,7 @@ portal_revoke_permissions (GDBusMethodInvocation *invocation, } do_set_permissions (entry, id, target_app_id, - ~perms & xdp_get_permissions (entry, target_app_id)); + ~perms & xdp_entry_get_permissions (entry, target_app_id)); g_dbus_method_invocation_return_value (invocation, g_variant_new ("()")); } @@ -210,7 +212,7 @@ portal_delete (GDBusMethodInvocation *invocation, return; } - if (!xdp_has_permissions (entry, app_id, XDP_PERMISSION_FLAGS_DELETE)) + if (!xdp_entry_has_permissions (entry, app_id, XDP_PERMISSION_FLAGS_DELETE)) { g_dbus_method_invocation_return_error (invocation, XDG_APP_ERROR, XDG_APP_ERROR_NOT_ALLOWED, "Not enough permissions"); @@ -226,13 +228,19 @@ portal_delete (GDBusMethodInvocation *invocation, } char * -do_create_doc (const char *path) +do_create_doc (struct stat *parent_st_buf, const char *path) { - g_autoptr(GVariant) data = g_variant_ref_sink (g_variant_new_bytestring (path)); + g_autoptr(GVariant) data = NULL; g_autoptr (XdgAppDbEntry) entry = NULL; g_auto(GStrv) ids = NULL; char *id = NULL; + data = + g_variant_ref_sink (g_variant_new ("(^aytt)", + path, + (guint64)parent_st_buf->st_dev, + (guint64)parent_st_buf->st_ino)); + ids = xdg_app_db_list_ids_by_value (db, data); if (ids[0] != NULL) @@ -267,48 +275,19 @@ static void portal_add (GDBusMethodInvocation *invocation, GVariant *parameters, const char *app_id) -{ - const char *path; - g_autofree char *id = NULL; - - if (app_id[0] != '\0') - { - /* don't allow this from within the sandbox */ - g_dbus_method_invocation_return_error (invocation, - XDG_APP_ERROR, XDG_APP_ERROR_NOT_ALLOWED, - "Not allowed inside sandbox"); - return; - } - - g_variant_get (parameters, "(^ay)", &path); - if (!g_path_is_absolute (path)) - { - g_dbus_method_invocation_return_error (invocation, - XDG_APP_ERROR, XDG_APP_ERROR_INVALID_ARGUMENT, - "Document paths must be absolute"); - return; - } - - id = do_create_doc (path); - - g_dbus_method_invocation_return_value (invocation, - g_variant_new ("(s)", id)); -} - -static void -portal_add_local (GDBusMethodInvocation *invocation, - GVariant *parameters, - const char *app_id) { GDBusMessage *message; GUnixFDList *fd_list; g_autofree char *id = NULL; g_autofree char *proc_path = NULL; int fd_id, fd, fds_len, fd_flags; + glnx_fd_close int dir_fd = -1; const int *fds; char path_buffer[PATH_MAX+1]; ssize_t symlink_size; - struct stat st_buf, real_st_buf; + struct stat st_buf, real_st_buf, real_parent_st_buf; + g_autofree char *dirname = NULL; + g_autofree char *name = NULL; g_variant_get (parameters, "(h)", &fd_id); @@ -326,15 +305,18 @@ portal_add_local (GDBusMethodInvocation *invocation, proc_path = g_strdup_printf ("/proc/self/fd/%d", fd); if (fd == -1 || + /* Must be able to get fd flags */ + (fd_flags = fcntl (fd, F_GETFL)) == -1 || + /* Must be O_PATH */ + ((fd_flags & O_PATH) != O_PATH) || + /* Must not be O_NOFOLLOW (because we want the target file) */ + ((fd_flags & O_NOFOLLOW) == O_PATH) || /* Must be able to fstat */ fstat (fd, &st_buf) < 0 || /* Must be a regular file */ (st_buf.st_mode & S_IFMT) != S_IFREG || - /* Must be able to get fd flags */ - (fd_flags = fcntl (fd, F_GETFL)) == -1 || - /* Must be able to read */ - (fd_flags & O_ACCMODE) == O_WRONLY || /* Must be able to read path from /proc/self/fd */ + /* This is an absolute and (at least at open time) symlink-expanded path */ (symlink_size = readlink (proc_path, path_buffer, sizeof (path_buffer) - 1)) < 0) { g_dbus_method_invocation_return_error (invocation, @@ -345,7 +327,15 @@ portal_add_local (GDBusMethodInvocation *invocation, path_buffer[symlink_size] = 0; - if (lstat (path_buffer, &real_st_buf) < 0 || + /* We open the parent directory and do the stat in that, so that we have + trustworthy parent dev/ino for later verification. Otherwise the caller + could later replace a parent with a symlink and make us read some other file */ + dirname = g_path_get_dirname (path_buffer); + name = g_path_get_basename (path_buffer); + dir_fd = open (dirname, O_CLOEXEC|O_PATH); + + if (fstat (dir_fd, &real_parent_st_buf) < 0 || + fstatat (dir_fd, name, &real_st_buf, AT_SYMLINK_NOFOLLOW) < 0 || st_buf.st_dev != real_st_buf.st_dev || st_buf.st_ino != real_st_buf.st_ino) { @@ -356,19 +346,17 @@ portal_add_local (GDBusMethodInvocation *invocation, return; } - id = do_create_doc (path_buffer); + id = do_create_doc (&real_parent_st_buf, path_buffer); if (app_id[0] != '\0') { - /* also grant app-id perms based on file_mode */ - guint32 perms = XDP_PERMISSION_FLAGS_GRANT_PERMISSIONS | XDP_PERMISSION_FLAGS_READ; g_autoptr(XdgAppDbEntry) entry = NULL; entry = xdg_app_db_lookup (db, id); - if ((fd_flags & O_ACCMODE) == O_RDWR) - perms |= XDP_PERMISSION_FLAGS_WRITE; - - do_set_permissions (entry, id, app_id, perms); + do_set_permissions (entry, id, app_id, + XDP_PERMISSION_FLAGS_GRANT_PERMISSIONS | + XDP_PERMISSION_FLAGS_READ | + XDP_PERMISSION_FLAGS_WRITE); } g_dbus_method_invocation_return_value (invocation, @@ -428,7 +416,6 @@ on_bus_acquired (GDBusConnection *connection, g_signal_connect_swapped (helper, "handle-get-mount-point", G_CALLBACK (handle_get_mount_point), NULL); g_signal_connect_swapped (helper, "handle-add", G_CALLBACK (handle_method), portal_add); - g_signal_connect_swapped (helper, "handle-add-local", G_CALLBACK (handle_method), portal_add_local); g_signal_connect_swapped (helper, "handle-grant-permissions", G_CALLBACK (handle_method), portal_grant_permissions); g_signal_connect_swapped (helper, "handle-revoke-permissions", G_CALLBACK (handle_method), portal_revoke_permissions); g_signal_connect_swapped (helper, "handle-delete", G_CALLBACK (handle_method), portal_delete); diff --git a/document-portal/xdp-util.c b/document-portal/xdp-util.c index d838d75..d0c6d59 100644 --- a/document-portal/xdp-util.c +++ b/document-portal/xdp-util.c @@ -50,7 +50,7 @@ xdp_parse_permissions (const char **permissions) } XdpPermissionFlags -xdp_get_permissions (XdgAppDbEntry *entry, +xdp_entry_get_permissions (XdgAppDbEntry *entry, const char *app_id) { g_autofree const char **permissions = NULL; @@ -63,13 +63,13 @@ xdp_get_permissions (XdgAppDbEntry *entry, } gboolean -xdp_has_permissions (XdgAppDbEntry *entry, +xdp_entry_has_permissions (XdgAppDbEntry *entry, const char *app_id, XdpPermissionFlags perms) { XdpPermissionFlags current_perms; - current_perms = xdp_get_permissions (entry, app_id); + current_perms = xdp_entry_get_permissions (entry, app_id); return (current_perms & perms) == perms; } @@ -87,28 +87,92 @@ xdp_name_from_id (guint32 doc_id) } const char * -xdp_get_path (XdgAppDbEntry *entry) +xdp_entry_get_path (XdgAppDbEntry *entry) { g_autoptr(GVariant) v = xdg_app_db_entry_get_data (entry); - return g_variant_get_bytestring (v); + g_autoptr(GVariant) c = g_variant_get_child_value (v, 0); + return g_variant_get_bytestring (c); } char * -xdp_dup_basename (XdgAppDbEntry *entry) +xdp_entry_dup_basename (XdgAppDbEntry *entry) { - const char *path = xdp_get_path (entry); + const char *path = xdp_entry_get_path (entry); return g_path_get_basename (path); } char * -xdp_dup_dirname (XdgAppDbEntry *entry) +xdp_entry_dup_dirname (XdgAppDbEntry *entry) { - const char *path = xdp_get_path (entry); + const char *path = xdp_entry_get_path (entry); return g_path_get_dirname (path); } +guint64 +xdp_entry_get_device (XdgAppDbEntry *entry) +{ + g_autoptr(GVariant) v = xdg_app_db_entry_get_data (entry); + g_autoptr(GVariant) c = g_variant_get_child_value (v, 1); + return g_variant_get_uint64 (c); +} + +guint64 +xdp_entry_get_inode (XdgAppDbEntry *entry) +{ + g_autoptr(GVariant) v = xdg_app_db_entry_get_data (entry); + g_autoptr(GVariant) c = g_variant_get_child_value (v, 2); + return g_variant_get_uint64 (c); +} + +int +xdp_entry_open_dir (XdgAppDbEntry *entry) +{ + g_autofree char *dirname = xdp_entry_dup_dirname (entry); + struct stat st_buf; + int fd; + + fd = open (dirname, O_CLOEXEC | O_PATH | O_DIRECTORY); + if (fd == -1) + return -1; + + if (fstat (fd, &st_buf) < 0) + { + close (fd); + errno = ENOENT; + return -1; + } + + if (st_buf.st_ino != xdp_entry_get_inode (entry) || + st_buf.st_dev != xdp_entry_get_device (entry)) + { + close (fd); + errno = ENOENT; + return -1; + } + + return fd; +} + +int +xdp_entry_stat (XdgAppDbEntry *entry, + struct stat *buf, + int flags) +{ + glnx_fd_close int fd = -1; + g_autofree char *basename = xdp_entry_dup_basename (entry); + + fd = xdp_entry_open_dir (entry); + if (fd < 0) + return -1; + + if (fstatat (fd, basename, buf, flags) != 0) + return -1; + + return 0; +} + static GHashTable *app_ids; typedef struct { diff --git a/document-portal/xdp-util.h b/document-portal/xdp-util.h index 3fa8de1..cb67817 100644 --- a/document-portal/xdp-util.h +++ b/document-portal/xdp-util.h @@ -9,16 +9,25 @@ G_BEGIN_DECLS const char ** xdg_unparse_permissions (XdpPermissionFlags permissions); XdpPermissionFlags xdp_parse_permissions (const char **permissions); -XdpPermissionFlags xdp_get_permissions (XdgAppDbEntry *entry, - const char *app_id); -gboolean xdp_has_permissions (XdgAppDbEntry *entry, - const char *app_id, - XdpPermissionFlags perms); -const char * xdp_get_path (XdgAppDbEntry *entry); -char * xdp_dup_basename (XdgAppDbEntry *entry); -char * xdp_dup_dirname (XdgAppDbEntry *entry); -guint32 xdp_id_from_name (const char *name); -char * xdp_name_from_id (guint32 doc_id); + +XdpPermissionFlags xdp_entry_get_permissions (XdgAppDbEntry *entry, + const char *app_id); +gboolean xdp_entry_has_permissions (XdgAppDbEntry *entry, + const char *app_id, + XdpPermissionFlags perms); +const char * xdp_entry_get_path (XdgAppDbEntry *entry); +char * xdp_entry_dup_basename (XdgAppDbEntry *entry); +char * xdp_entry_dup_dirname (XdgAppDbEntry *entry); +guint64 xdp_entry_get_device (XdgAppDbEntry *entry); +guint64 xdp_entry_get_inode (XdgAppDbEntry *entry); +int xdp_entry_open_dir (XdgAppDbEntry *entry); +int xdp_entry_stat (XdgAppDbEntry *entry, + struct stat *buf, + int flags); + +guint32 xdp_id_from_name (const char *name); +char * xdp_name_from_id (guint32 doc_id); + void xdp_invocation_lookup_app_id (GDBusMethodInvocation *invocation, GCancellable *cancellable, -- cgit v1.2.1