Created attachment 103204 [details] Gzipped tar for demonstration When an archive includes an executable shell script and the user chose to open files in their associated applications, clicking on those scripts runs them. As there is no indication (except the small icon in some cases) of the file being executable, this is highly misleading and even dangerous. The attachment shows the issue, the LICENSE file gets executed when opening it.
Thanks for the report, this is quite serious.
Git commit 82fdfd24d46966a117fa625b68784735a40f9065 by Elvis Angelaccio. Committed on 06/01/2017 at 14:35. Pushed by elvisangelaccio into branch 'Applications/16.12'. Stop running executables when opening urls This is a security risk because it's not clear when an entry in an archive is an executable. FIXED-IN: 16.12.1 M +1 -1 part/part.cpp https://commits.kde.org/ark/82fdfd24d46966a117fa625b68784735a40f9065
Git commit 49ce94df19607e234525afda5ad4190ce35300c3 by Elvis Angelaccio. Committed on 06/01/2017 at 15:04. Pushed by elvisangelaccio into branch 'Applications/16.08'. Stop running executables when opening urls This is a security risk because it's not clear when an entry in an archive is an executable. M +1 -1 part/part.cpp https://commits.kde.org/ark/49ce94df19607e234525afda5ad4190ce35300c3
Thanks for fixing! However, I'm not sure whether this is the right approach. What about a dialog that tells the user that it is an executable file and it should only be run if it's trusted and also that it may need the archive to be extraced somewhere? It's the approach that's used on Windows as well.
(In reply to Fabian Vogt from comment #4) > Thanks for fixing! > > However, I'm not sure whether this is the right approach. > What about a dialog that tells the user that it is an executable file and it > should only be run if it's trusted and also that it may need the archive to > be extraced somewhere? It's the approach that's used on Windows as well. Well we could do that, but only on master. Stable branches can do with my fix above. Anyway, I'm not sure if we can check the executable bit before extraction. What Ark currently does is extracting the selected entry to a temporary folder, then KRun is called over that url. So the dialog would only ask "you are about to run this file, do you trust it?"
(In reply to Elvis Angelaccio from comment #5) > (In reply to Fabian Vogt from comment #4) > > Thanks for fixing! > > > > However, I'm not sure whether this is the right approach. > > What about a dialog that tells the user that it is an executable file and it > > should only be run if it's trusted and also that it may need the archive to > > be extraced somewhere? It's the approach that's used on Windows as well. > > Well we could do that, but only on master. Stable branches can do with my > fix above. I haven't tested the fix yet, what happens now if the user clicks on the file, is it opened in a text editor, so as-if the file weren't executable? If nothing happens at all, I'd consider it as a regression, even. > Anyway, I'm not sure if we can check the executable bit before extraction. > What Ark currently does is extracting the selected entry to a temporary > folder, then KRun is called over that url. So the dialog would only ask "you > are about to run this file, do you trust it?" Couldn't you test the executable flag between extraction and execution?
(In reply to Fabian Vogt from comment #6) > (In reply to Elvis Angelaccio from comment #5) > > (In reply to Fabian Vogt from comment #4) > > > Thanks for fixing! > > > > > > However, I'm not sure whether this is the right approach. > > > What about a dialog that tells the user that it is an executable file and it > > > should only be run if it's trusted and also that it may need the archive to > > > be extraced somewhere? It's the approach that's used on Windows as well. > > > > Well we could do that, but only on master. Stable branches can do with my > > fix above. > > I haven't tested the fix yet, what happens now if the user clicks on the > file, > is it opened in a text editor, so as-if the file weren't executable? Yes. > > Couldn't you test the executable flag between extraction and execution? Yes, that's what I meant. Check the flag and show the dialog if necessary.
Actually there is another issue with the dialog approach: if there is no associated application (e.g. the file is a binary executable), then KRun shows another dialog: "The file file:///xxx is an executable program. For safety it will not be started." So this scenario would be a bit annoying: - select binary executable and click Open - ark asks: "this is an exe, do you trust it"? (1st dialog) - user chooses No - KRun says: "The file ... is an executable program. For safety it will not be started." (2nd dialog)
(In reply to Elvis Angelaccio from comment #8) > Actually there is another issue with the dialog approach: if there is no > associated application (e.g. the file is a binary executable), then KRun > shows another dialog: > > "The file file:///xxx is an executable program. For safety it will not be > started." > > So this scenario would be a bit annoying: > - select binary executable and click Open > - ark asks: "this is an exe, do you trust it"? (1st dialog) > - user chooses No > - KRun says: "The file ... is an executable program. For safety it will not > be started." (2nd dialog) In that case it's probably the best if clicking on No aborts the whole process instead, if KRun::isExecutable returns true for the mime type.
Honestly, i would simply and plainly not execute anything, the option is called "Open" not "Run", one could argue that "opening" and executable is running it, but really, in which scenario does running an executable from inside a tarball without extracting the rest is going to work? I'd vote for keeping it simple, at most if it's an executable raise a warning saying "this is an executable we will not run it", because it's also true that pressing a button and getting nothing back is weird.
Git commit 6b6da3f2e6ac5ca12b46d208d532948c1dbb8776 by Elvis Angelaccio. Committed on 09/01/2017 at 20:29. Pushed by elvisangelaccio into branch 'Applications/16.04'. Stop running executables when opening urls This is a security risk because it's not clear when an entry in an archive is an executable. M +1 -1 part/part.cpp https://commits.kde.org/ark/6b6da3f2e6ac5ca12b46d208d532948c1dbb8776
Git commit e2448360eca1b81eb59fffca9584b0fc5fbd8e5b by Elvis Angelaccio. Committed on 09/01/2017 at 20:34. Pushed by elvisangelaccio into branch 'Applications/15.12'. Stop running executables when opening urls This is a security risk because it's not clear when an entry in an archive is an executable. M +1 -1 part/part.cpp https://commits.kde.org/ark/e2448360eca1b81eb59fffca9584b0fc5fbd8e5b
Albert makes a good point. I'll close this report for now, we can change the behavior on master if a significant number of people asks for it.