flathub / org.gnome.Evince

https://flathub.org/apps/details/org.gnome.Evince
2 stars 15 forks source link

Sandbox this harder (instead of filesystem=host permissions by default)? #76

Closed nekohayo closed 1 year ago

nekohayo commented 2 years ago

It was brought to my attention that poppler and ghostscript can be security concerns, and that Evince should therefore be run under sandboxes.

Looking at it with flatseal, I see that:

hadess commented 2 years ago
  • Evince has filesystem=host permissions by default. Shouldn't it be restricted to access only xdg-download and xdg-documents by default, or at best filesystem=home?

That's for the "recent files view" that you get when running "evince" to still work correctly.

  • It has "socket=pulseaudio"... why would it need that?

Because there can be videos in PDFs. That's implemented in libview/ev-media-player.c.

gpoo commented 2 years ago

FWIW, the flatpak version of Evince does not ship the backends for DVI (tetex-live) nor PS (ghostshipt).

gpoo commented 2 years ago

Quoting what I wrote in https://github.com/flathub/org.gnome.Evince/pull/61#issuecomment-939322184

I dislike having filesystem=host enabled, but IIUC, that is the one the currently gives Evince access to neighbour files. That is, until there is a fix for https://github.com/flatpak/xdg-desktop-portal/issues/463.

Mikenux commented 2 years ago

Neighbor files will work in most cases, but not all. This will not work if you have a summary pdf and then folders where other pdf files are located if you first open a pdf located in one of those folders. If you do this, the opened pdf file will not be able to access the summary because it will not be in the same directory.

Example:

Directly open issue1-summary.pdf in Issue1Folder. Grant access to neighboring files in this folder. If issue1-summary has an entry to access GeneralSummary.pdf, it won't work.

There is another question: If I open GeneralSummary, then open issue-1-summary from it, then close GeneralSummary, will access to GeneralSummary still be available or will it be forgotten?

However, I don't think this is a common case.

hadess commented 2 years ago

However, I don't think this is a common case.

I've never ever seen a PDF that linked to other PDFs on the local filesystem. I've seen it on the web...

Mikenux commented 2 years ago

However, I don't think this is a common case.

I've never ever seen a PDF that linked to other PDFs on the local filesystem. I've seen it on the web...

https://gitlab.gnome.org/GNOME/evince/-/issues/48. It was on DVD, but the author of the issue said it took up hard drive space. This was an old issue, but in the comments someone else used a manual and he used Fedora 35. As for the DVD, you can copy the files to the hard drive for storage.

Audio files can also be in a folder (see https://gitlab.gnome.org/GNOME/evince/-/issues/1584). Do the folders fit into the neighboring files case?

mwleeds commented 2 years ago
  • Evince has filesystem=host permissions by default. Shouldn't it be restricted to access only xdg-download and xdg-documents by default, or at best filesystem=home?

That's for the "recent files view" that you get when running "evince" to still work correctly.

Why though? I just tested removing filesystem=host using Flatseal, opened a PDF using the "Open..." button, closed Evince, opened Evince again, and the recent document is there and works. The file access is persistent:

$ flatpak documents org.gnome.Evince 
ID
f77477f2
hadess commented 2 years ago

Why though?

In my memory, the recent files view showed all the PDFs, not just the ones opened by Evince. That might not be accurate (anymore?).

Mikenux commented 2 years ago

In my memory, the recent files view showed all the PDFs, not just the ones opened by Evince. That might not be accurate (anymore?).

The recent view only shows files recently opened by Evince (I tested removing the filesystem=host permission and two files, and both files are shown in the recent view). Thus, access can be limited. I guess if flatpak goes with file types/extensions globally (for all files in home/user), it would be an improvement even without the restriction with neighboring files - at least if it's doable.

mwleeds commented 2 years ago

Why though?

In my memory, the recent files view showed all the PDFs, not just the ones opened by Evince. That might not be accurate (anymore?).

Looks like Evince ignores recent items from other applications, since at least 2014: https://gitlab.gnome.org/GNOME/evince/-/blob/b4bdbc420c8d7bec29a710a43deb8e8368275b8d/shell/ev-recent-view.c#L717

Here's an MR to remove the filesystem=host permission: https://gitlab.gnome.org/GNOME/evince/-/merge_requests/462

mwleeds commented 2 years ago

I guess if flatpak goes with file types/extensions globally (for all files in home/user), it would be an improvement even without the restriction with neighboring files - at least if it's doable.

I don't understand what you mean, could you try rephrasing?

Mikenux commented 2 years ago

@mwleeds:

There is discussion about limiting access to specific file types (e.g. images, videos) and file extensions that are in a given folder (for example, automatically open a subtitle file when a video file is opened; see https://github.com/flatpak/xdg-desktop-portal/issues/463).

What I said is that this access limitation could also be applied in general, i.e. when the application has access to all folders.

Mikenux commented 2 years ago

If the neighboring files functionality is designed to be simple (i.e. without giving the reason for accessing these files), I think it doesn't make sense to have a dialog asking for it when, for example, a PDF file is opened... The question is whether users will click "Allow" if they don't know why Evince wants to access neighboring files when opening their PDF file (is it obvious when a user has a catalog of PDFs or has downloaded a PDF with additional resources?).

Mikenux commented 2 years ago

@hadess:

Given what I've read and commented on in another issue of Evince, I think neighboring files should (also) be available as a filesystem restriction to have regular filesystem names − and not the sandboxed ones, needed for "Open Containing Folder".

hadess commented 2 years ago

Given what I've read and commented on in another issue of Evince, I think neighboring files should (also) be available as a filesystem restriction to have regular filesystem names − and not the sandboxed ones, needed for "Open Containing Folder".

I can't parse this, sorry.

Mikenux commented 2 years ago

If filesystem=host is specified, when we click "Open Containing Folder", Evince shows the file in its original folder (e.g. /home/user/Documents/MyFiles - the "normal path") .

If filesystem=host is disabled (via Flatseal), Evince is more sandboxed, and clicking "Open Containing Folder" shows the file in a sandboxed folder (e.g. /run/user/1000/doc/ a1b23c4d - the "sandbox path").

So if the neighboring files feature creates a sandbox for the files, Evince will have a sandbox path for them. And I think we want the normal path when we click "Open Containing Folder".

hadess commented 2 years ago

I honestly don't know what you want me to do with this information. Is it a bug report, or ideas for a design for a portal feature that doesn't yet exist?

mwleeds commented 2 years ago

Sounds to me like a comment that should be added to https://github.com/flatpak/xdg-desktop-portal/issues/463 so it can then either be taken into account in the design of that portal, or moved to a separate issue on the xdg-desktop-portal repo for later follow-up.

Mikenux commented 2 years ago

It's just that if the neighboring files portal creates a sandbox, and if Evince uses it, "Open Containing Folder" will open a sandboxed folder instead of showing the correct location.

This was mentioned on the neighboring files issue when we thought it would be nice to have it as an improvement over "filesystem=host". But it probably wasn't clear. It's hard to say something when we don't know how this portal will be developed...

Mikenux commented 2 years ago

Given that this is the case when Evince does not use filesystem=host and in Firefox, I decided to open a bug (https://github.com/flatpak/xdg-desktop-portal/issues/807).

hadess commented 2 years ago
  • Evince has filesystem=host permissions by default. Shouldn't it be restricted to access only xdg-download and xdg-documents by default, or at best filesystem=home?

I just remembered that the host filesystem was probably so that evince could access system-provided documentation. That should still work through the document portal though.

rugk commented 2 years ago

Stumbled upon the same and first wrongly attributed it to an update in GNOME Software showing an "added" file system permission.

gpoo commented 1 year ago

https://github.com/flathub/org.gnome.Evince/pull/88 fixed the issue.

julianfairfax commented 1 year ago

The home permission is still quite vast. Why does it need access to the entire home folder? As far as I'm aware, it can open any file through a portal. It doesn't need constant access to a user's whole home folder?

rugk commented 1 year ago

I agree, especially if you consider software vulnerabilities like in WebP currently (which gets quite some attention/media coverage), you may have similar stuff when parsing PDFs.

As such Evince is quite a high-risk target and IMHO you should sandbox it harder again. Technically using a portal would be the best.

A6GibKm commented 1 year ago

Please if you want to comment read the previous comments. The app has read-only accessto the home and it needs for features as synctex to work. Note that without write or network access there is no much that a vulnerability can do.

Mikenux commented 1 year ago

Yes. And this is also necessary for PDFs containing links to external files (e.g. audio, videos, other PDFs).

rugk commented 1 year ago

Note that without write or network access there is no much that a vulnerability can do.

But it has write access, does not it? Ahhh, oh I just saw it is home:ro okay then yeah, it is way better. Anyway, stricter sandboxing and having basic functionality retained would be awesome, e.g. last time I tried without having the filesystem permission I could not save a file (aka the added comments and notes).

PDFs containing links to external files (e.g. audio, videos, other PDFs).

Well unless it is embedded links should be opened by the linkhandler, should not it?

Also I wonder about that advanced features that have been mentioned like synctex are likely not the majority of use cases. Also things like embedding stuff in your PDF from other files, I mean, this sounds not good… :upside_down_face:

Mikenux commented 1 year ago

Well unless it is embedded links should be opened by the linkhandler, should not it?

Yes. However, for example, PDF files intended for presentations may contain links that point to audio and video files external to the PDF (if done correctly by the PDF author, these files are in the same folder as the PDF file) , but presented without leaving the PDF (so played inside the PDF).

Even though this case and the synctex case do not constitute the majority of use cases, what developers generally do is ensure that their applications are able to use all their features in order to satisfy all their users .

That said, no one is stopping you from sandboxing Evince harder by removing relevant permissions if you don't have such use cases. Note that this makes "Open Containing Folder" open the file in the sandbox location (path with /run/user/...).

rugk commented 1 year ago

FYI BTW https://github.com/flatpak/xdg-desktop-portal/issues/477#issuecomment-1749183406 has been solved, in case this helps/applies here.