Comment 4 for bug 1750069

Revision history for this message
Seth Arnold (seth-arnold) wrote :

I reviewed xdg-desktop-portal-gtk version 0.11-1 as checked into cosmic.
This isn't a full security audit but rather a quick gauge of
maintainability.

xdg-desktop-portal-gtk is a "backend" for the portal system to try to make
linux namespacing more ergonomic. It provides dialogs that serve dual
purposes: the standard file-pickers, etc., while simultaneously serving as
unobtrusive access control tools. (Aka "powerbox".)

- No CVE history

- xdg-desktop-portal-gtk is the user-facing portion of the portals
  toolkit; sandboxed applications will use xdk-desktop-portal to call
  into this package or other similar ones for different environments,
  users will make access control decisions, and the results will be
  passed back into the sandboxed applications.

  All the interactions are handled over dbus.

- Build-Depends: dbus, debhelper, libdbus-1-dev, libglib2.0-dev,
  libgtk-3-dev, xdg-desktop-portal-dev, xmlto
- Does not itself do networking
- No pre/post inst/rm scripts
- No init scripts
- systemd user unit file to start
  /usr/lib/xdg-desktop-portal/xdg-desktop-portal-gtk on the dbus service
  org.freedesktop.impl.portal.desktop.gtk
- No setuid files
- No binaries in PATH
- No sudo fragments
- No udev rules
- No test suite
- No cron jobs
- Build logs have some errors

- subprocesses are spawned. The launch_preview() function looks unsafe and
  may need a CVE. The mail compose methods probably allow a malicious
  file to exfiltrate data off the system if the operator isn't paying
  close attention.

- memory management looked careful
- logging looked careful
- sets GIO_USE_VFS environment variable
- No cryptography
- Does not itself do networking, gnome vfs might
- Privileged vs unprivileged portions of code are difficult to untangle
  via casual inspection; I believe this entire package is privileged, but
  I'm not sure if filenames, inputs, etc., are therefore completely
  trusted or completely untrusted or somewhere in the middle.
- No temporary files
- No WebKit
- No PolKit

Here's some of the messy logs:

update-rc.d: warning: start and stop actions are no longer supported; falling back to defaults
/usr/include/glib-2.0/glib/gmem.h:124:8: warning: mutter_session_proxy may be used uninitialized in this function [-Wmaybe-uninitialized]
src/remotedesktopdialog.c:148:16: warning: device_type_name may be used uninitialized in this function [-Wmaybe-uninitialized]
dh_install: Please use dh_missing --list-missing/--fail-missing instead
E: Lintian run failed (policy violation)
Lintian: fail

- image_button_clicked() does image previews
- compose_mail_thunderbird() and compose_mail_evolution() would probably
  allow attaching arbitrary files via malicious addresses -- are the
  addresses shown specifically to the user to confirm them first? The
  thunderbird variant may also allow the same attack via subject and
  body text.

- supports
  file chooser
  app chooser
  print
  screenshot
  notification
  inhibit
  access
  account
  email
  screen cast
  remote desktop

- launch_preview() appears to use unsafe string-based execution with
  user-supplied content rather than safe array-based execution.

A trusted helper tool like this is probably going to be an important
part of Linux safety and security in the future. I'm worried that this
implementation relies upon dbus, which is not particularly simple, and
provides ready access to a wide array of extremely "porous" targets --
mail user agents, evince, thumbnailing, etc., and appears to have made
some classic security programming mistakes itself.

These tools need more review by more reviewers.

Security team ACK for promoting xdg-desktop-portal-gtk to main for Cosmic,
but not yet for Bionic or previous LTS releases.

Thanks