Dear all, This is an important topic since it involves security. I'd appreciate it if you spent some time on understanding the issue.
I see three options for what to do about the minted + shell-escape issue: 1. Revert the recently added minted support. 2. Keep the current state of master. 3. Apply the patch at [1] (also attached for convenience). We do not have unanimous agreement on what to do and I would like to have a vote, since this topic involves security. Another relevant piece of news is that the minted author is interested in making it so -shell-escape is not needed [2]. That work could be done in minted within a few months (see the Github issue for details), and perhaps we could incorporate this into LyX 2.4.0. For more background, see the mega-thread at [3]. See, in particular, the useful summary from Guillaume's perspective at [4]. I can see reasonable arguments for all three options. When voting, I ask you to focus specifically on the issue of security. Although adding minted support is a nice feature, if we think it will decrease security, we should not do it. The question I focus on when deciding which option I think is best is "with which option is the average user least likely to end up running malicious code"? Scott [1] https://www.mail-archive.com/search?l=mid&q=20170620203439.GC1008%40GIOVE [2] https://github.com/gpoore/minted/issues/166 [3] https://www.mail-archive.com/search?l=mid&q=20170529215308.ojd3b42tkyqmnl4l%40steph [4] https://www.mail-archive.com/search?l=mid&q=oi9r49%24hn2%241%40blaine.gmane.org
diff --git a/src/Converter.cpp b/src/Converter.cpp
index 6e10b18704..45b725e8c6 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -19,14 +19,18 @@
#include "Encoding.h"
#include "ErrorList.h"
#include "Format.h"
+#include "InsetList.h"
#include "Language.h"
#include "LaTeX.h"
#include "LyXRC.h"
#include "Mover.h"
+#include "ParagraphList.h"
#include "Session.h"
#include "frontends/alert.h"
+#include "insets/InsetInclude.h"
+
#include "support/debug.h"
#include "support/FileNameList.h"
#include "support/filetools.h"
@@ -279,20 +283,38 @@ OutputParams::FLAVOR
Converters::getFlavor(Graph::EdgePath const & path,
}
-bool Converters::checkAuth(Converter const & conv, string const & doc_fname)
+bool Converters::checkAuth(Converter const & conv, string const & doc_fname,
+ bool use_shell_escape)
{
- if (!conv.need_auth())
+ if (!conv.latex())
+ use_shell_escape = false;
+ if (!conv.need_auth() && !use_shell_escape)
return true;
- const docstring security_warning = bformat(
- _("<p>The requested operation requires the use of a converter
from "
- "%2$s to %3$s:</p>"
+ string conv_command = conv.command();
+ bool const has_shell_escape = contains(conv_command, "-shell-escape");
+ size_t const token_pos = conv_command.find("$$");
+ bool const has_token = token_pos != string::npos;
+ string const command = use_shell_escape && !has_shell_escape
+ ? (has_token ? conv_command.insert(token_pos, "-shell-escape ")
+ : conv_command.append(" -shell-escape"))
+ : conv_command;
+ docstring const security_warning = (use_shell_escape
+ ? bformat(_("<p>This document uses minted listings, so the LaTeX "
+ "backend needs to be able to launch external programs:</p>"
+ "<center><p><tt>%1$s</tt></p></center>"
+ "<p>The external programs can execute arbitrary commands on "
+ "your system, including dangerous ones, if instructed to do "
+ "so by a maliciously crafted LyX document.</p>"),
+ from_utf8(command))
+ : bformat(_("<p>The requested operation requires the use of a "
+ "converter from %2$s to %3$s:</p>"
"<blockquote><p><tt>%1$s</tt></p></blockquote>"
- "<p>This external program can execute arbitrary commands on
your "
- "system, including dangerous ones, if instructed to do so by a "
- "maliciously crafted .lyx document.</p>"),
- from_utf8(conv.command()), from_utf8(conv.from()),
- from_utf8(conv.to()));
- if (lyxrc.use_converter_needauth_forbidden) {
+ "<p>This external program can execute arbitrary commands on "
+ "your system, including dangerous ones, if instructed to do "
+ "so by a maliciously crafted LyX document.</p>"),
+ from_utf8(command), from_utf8(conv.from()),
+ from_utf8(conv.to())));
+ if (lyxrc.use_converter_needauth_forbidden && !use_shell_escape) {
frontend::Alert::error(
_("An external converter is disabled for security reasons"),
security_warning + _(
@@ -302,29 +324,43 @@ bool Converters::checkAuth(Converter const & conv, string
const & doc_fname)
"Forbid needauth converters</i>.)"), false);
return false;
}
- if (!lyxrc.use_converter_needauth)
+ if (!lyxrc.use_converter_needauth && !use_shell_escape)
return true;
- static const docstring security_title =
- _("An external converter requires your authorization");
+ docstring const security_title = use_shell_escape
+ ? _("A LaTeX backend requires your authorization")
+ : _("An external converter requires your authorization");
int choice;
- const docstring security_warning2 = security_warning +
- _("<p>Would you like to run this converter?</p>"
- "<p><b>Only run if you trust the origin/sender of the LyX "
- "document!</b></p>");
+ docstring const security_warning2 = security_warning + (use_shell_escape
+ ? _("<p>Should LaTeX backends be allowed to run external "
+ "programs?</p><p><b>Allow them only if you trust the "
+ "origin/sender of the LyX document!</b></p>")
+ : _("<p>Would you like to run this converter?</p>"
+ "<p><b>Only run if you trust the origin/sender of the LyX "
+ "document!</b></p>"));
+ docstring const no = use_shell_escape
+ ? _("Do ¬ allow") : _("Do ¬ run");
+ docstring const yes = use_shell_escape ? _("A&llow") : _("&Run");
+ docstring const always = use_shell_escape
+ ? _("&Always allow for this document")
+ : _("&Always run for this document");
if (!doc_fname.empty()) {
LYXERR(Debug::FILES, "looking up: " << doc_fname);
- std::set<std::string> & auth_files =
theSession().authFiles().authFiles();
+ std::set<std::string> & auth_files = use_shell_escape
+ ? theSession().shellescapeFiles().shellescapeFiles()
+ : theSession().authFiles().authFiles();
if (auth_files.find(doc_fname) == auth_files.end()) {
- choice = frontend::Alert::prompt(security_title,
security_warning2,
- 0, 0, _("Do ¬ run"), _("&Run"), _("&Always
run for this document"));
+ choice = frontend::Alert::prompt(security_title,
+ security_warning2,
+ 0, 0, no, yes, always);
if (choice == 2)
auth_files.insert(doc_fname);
} else {
choice = 1;
}
} else {
- choice = frontend::Alert::prompt(security_title,
security_warning2,
- 0, 0, _("Do ¬ run"), _("&Run"));
+ choice = frontend::Alert::prompt(security_title,
+ security_warning2,
+ 0, 0, no, yes);
}
return choice != 0;
}
@@ -459,7 +495,30 @@ bool Converters::convert(Buffer const * buffer,
"tmpfile.out"));
}
- if (!checkAuth(conv, buffer ? buffer->absFileName() : string()))
+ bool use_shell_escape = false;
+ if (buffer && buffer->params().use_minted) {
+ // Check for minted listings insets
+ for (Paragraph const & par : buffer->paragraphs()) {
+ InsetList const & insets = par.insetList();
+ pos_type lstpos = insets.find(LISTINGS_CODE, 0);
+ pos_type incpos = insets.find(INCLUDE_CODE, 0);
+ if (incpos >= 0) {
+ InsetInclude const * include =
+ static_cast<InsetInclude *>
+ (insets.get(incpos));
+ if (include->params().getCmdName() !=
+ "inputminted") {
+ incpos = -1;
+ }
+ }
+ if (lstpos >= 0 || incpos >= 0) {
+ use_shell_escape = true;
+ break;
+ }
+ }
+ }
+ if (!checkAuth(conv, buffer ? buffer->absFileName() : string(),
+ use_shell_escape))
return false;
if (conv.latex()) {
@@ -470,6 +529,8 @@ bool Converters::convert(Buffer const * buffer,
command = subst(command, token_from, "");
command = subst(command, token_latex_encoding,
buffer->params().encoding().latexName());
+ if (use_shell_escape && !contains(command,
"-shell-escape"))
+ command += " -shell-escape ";
LYXERR(Debug::FILES, "Running " << command);
if (!runLaTeX(*buffer, command, runparams, errorList))
return false;
diff --git a/src/Converter.h b/src/Converter.h
index 1ea73279b2..1963980c4f 100644
--- a/src/Converter.h
+++ b/src/Converter.h
@@ -193,8 +193,14 @@ public:
/// able to execute arbitrary code, tagged with the 'needauth' option,
/// authorization is: always denied if
lyxrc.use_converter_needauth_forbidden
/// is enabled; always allowed if the lyxrc.use_converter_needauth
- /// is disabled; user is prompted otherwise
- bool checkAuth(Converter const & conv, std::string const & doc_fname);
+ /// is disabled; user is prompted otherwise.
+ /// However, if use_shell_escape is true and a LaTeX backend is
+ /// going to be executed, both lyxrc.use_converter_needauth and
+ /// lyxrc.use_converter_needauth_forbidden are ignored, because in
+ /// this case the backend has to be executed and LyX will add the
+ /// -shell-escape option, so that user consent is always needed.
+ bool checkAuth(Converter const & conv, std::string const & doc_fname,
+ bool use_shell_escape = false);
private:
///
diff --git a/src/Session.cpp b/src/Session.cpp
index 310aa74c9b..347426a0fe 100644
--- a/src/Session.cpp
+++ b/src/Session.cpp
@@ -35,6 +35,7 @@ string const sec_session = "[session info]";
string const sec_toolbars = "[toolbars]";
string const sec_lastcommands = "[last commands]";
string const sec_authfiles = "[auth files]";
+string const sec_shellescape = "[shell escape files]";
} // anon namespace
@@ -422,6 +423,8 @@ void Session::readFile()
lastCommands().read(is);
else if (tmp == sec_authfiles)
authFiles().read(is);
+ else if (tmp == sec_shellescape)
+ shellescapeFiles().read(is);
else
LYXERR(Debug::INIT, "LyX: Warning: unknown Session
section: " << tmp);
@@ -442,6 +445,7 @@ void Session::writeFile() const
lastCommands().write(os);
bookmarks().write(os);
authFiles().write(os);
+ shellescapeFiles().write(os);
} else
LYXERR(Debug::INIT, "LyX: Warning: unable to save Session: "
<< session_file);
@@ -480,4 +484,34 @@ void AuthFilesSection::write(ostream & os) const
}
+void ShellEscapeSection::read(istream & is)
+{
+ string s;
+ do {
+ char c = is.peek();
+ if (c == '[')
+ break;
+ getline(is, s);
+ c = s[0];
+ if (c == 0 || c == '#' || c == ' ' || !FileName::isAbsolute(s))
+ continue;
+
+ // read shellescape files
+ FileName const file(s);
+ if (file.exists() && !file.isDirectory())
+ shellescape_files_.insert(s);
+ else
+ LYXERR(Debug::INIT, "LyX: Warning: Ignore shellescape
file: " << s);
+ } while (is.good());
+}
+
+
+void ShellEscapeSection::write(ostream & os) const
+{
+ os << '\n' << sec_shellescape << '\n';
+ copy(shellescape_files_.begin(), shellescape_files_.end(),
+ ostream_iterator<std::string>(os, "\n"));
+}
+
+
}
diff --git a/src/Session.h b/src/Session.h
index f471f4d28e..1698ba3537 100644
--- a/src/Session.h
+++ b/src/Session.h
@@ -341,6 +341,27 @@ private:
};
+class ShellEscapeSection : SessionSection
+{
+public:
+ ///
+ explicit ShellEscapeSection() {};
+
+ ///
+ void read(std::istream & is);
+
+ ///
+ void write(std::ostream & os) const;
+
+ ///
+ std::set<std::string> & shellescapeFiles() { return shellescape_files_;
}
+
+private:
+ /// set of document files authorized for external conversion
+ std::set<std::string> shellescape_files_;
+};
+
+
class Session
{
public:
@@ -373,6 +394,10 @@ public:
AuthFilesSection & authFiles() { return auth_files; }
///
AuthFilesSection const & authFiles() const { return auth_files; }
+ ///
+ ShellEscapeSection & shellescapeFiles() { return shellescape_files; }
+ ///
+ ShellEscapeSection const & shellescapeFiles() const { return
shellescape_files; }
private:
friend class LyX;
@@ -402,6 +427,8 @@ private:
LastCommandsSection last_commands;
///
AuthFilesSection auth_files;
+ ///
+ ShellEscapeSection shellescape_files;
};
/// This is a singleton class. Get the instance.
diff --git a/src/frontends/qt4/GuiView.cpp b/src/frontends/qt4/GuiView.cpp
index a2ef74bf53..18debc73b6 100644
--- a/src/frontends/qt4/GuiView.cpp
+++ b/src/frontends/qt4/GuiView.cpp
@@ -50,6 +50,7 @@
#include "Format.h"
#include "FuncStatus.h"
#include "FuncRequest.h"
+#include "InsetList.h"
#include "Intl.h"
#include "Layout.h"
#include "Lexer.h"
@@ -66,6 +67,8 @@
#include "Toolbars.h"
#include "version.h"
+#include "insets/InsetInclude.h"
+
#include "support/convert.h"
#include "support/debug.h"
#include "support/ExceptionMessage.h"
@@ -2905,6 +2908,38 @@ bool GuiView::closeBuffer(Buffer & buf)
// buffer, we can close or release the child buffers here too.
bool success = true;
if (!closing_) {
+ if (buf.params().use_minted && buf.isClean()) {
+ std::set<std::string> & shellescape_files =
+ theSession().shellescapeFiles().shellescapeFiles();
+ std::set<std::string>::iterator it =
+ shellescape_files.find(buf.absFileName());
+ if (it != shellescape_files.end()) {
+ // Check for minted listings insets
+ bool found_minted_listings = false;
+ for (Paragraph const & par : buf.paragraphs()) {
+ InsetList const & insets =
par.insetList();
+ pos_type lstpos =
insets.find(LISTINGS_CODE, 0);
+ pos_type incpos =
insets.find(INCLUDE_CODE, 0);
+ if (incpos >= 0) {
+ InsetInclude const * include =
+ static_cast<InsetInclude *>
+ (insets.get(incpos));
+ if
(include->params().getCmdName()
+ != "inputminted") {
+ incpos = -1;
+ }
+ }
+ if (lstpos >= 0 && incpos >= 0) {
+ found_minted_listings = true;
+ break;
+ }
+ }
+ // If the document doesn't use minted listings
+ // revoke permission to use shell-escape
+ if (!found_minted_listings)
+ shellescape_files.erase(it);
+ }
+ }
ListOfBuffers clist = buf.getChildren();
ListOfBuffers::const_iterator it = clist.begin();
ListOfBuffers::const_iterator const bend = clist.end();
signature.asc
Description: PGP signature
