On Tue, Jul 18, 2017 at 07:26:23PM -0400, Richard Heck wrote: > On 07/18/2017 09:56 AM, Jürgen Spitzmüller wrote: > > Am Dienstag, den 18.07.2017, 15:39 +0200 schrieb Jean-Marc Lasgouttes: > >> Whi, not, maybe along with the names of the converters (features) > >> Sweave/gnuplot/minted present in current document and accepted by the > >> user. > > I would add a verbose tooltip when hovering the icon, something like > > > > ''' > > NOTE: Shell escape access granted. > > > > For this document, access to the -shell-escape feature has been granted > > for the following converters: ... > > > > Note that this is a potential security risk. Use only if you trust the > > source of this document. Please refer to sec. xx of the User Guide for > > details. > > > > To withdraw shell escape access, press this icon. > > ''' > > This seems a reasonable solution to me. It is not perfect, but nothing is. > > As I see it, the issue is that there are actually a wide variety of > reasons that users might want to > enable -shell-escape for various converters. As LyX currently functions, > the only way to do this > is to add this to the converter itself. This is dangerous from our point > of view NOT so much (or > only) because it is intrinsically dangerous, but rather because it it is > the kind of thing that is too > easy to "do and forget". Or, to put it differently: It is a serious > hassle to enable -shell-escape as > things are, and that invites people to do it and leave it. And that > really is a security risk.
The attached patch takes into account all of these ideas. As a disclaimer, note that I am providing it only because I am now familiar with this part of the code and can quickly come up with a patch. But I am not endorsing it. The user can choose to allow execution of external programs for a given document through the document settings. However, this is a property that only holds on the computer used to edit the document. There is no way to send out a document with the shell-escape thing activated. Once activated, the user is prompted for confirmation each time a latex backend is used, unless he decides to always allow execution for a particular document. A document marked as requiring shell escape privileges is characterized by a red icon on the status bar. The shell-escape privilege can be revoked through the document settings. Given the peculiarity that this cannot be a mere document property but rather a property tied to both document and computer, the check box works differently from all other check boxes. Indeed, checking or unchecking it should not make dirty the document. So, the privilege is given or revoked instantly when checking or unchecking, without the need of confirming the change. The patch also nags the user when -shell-escape is added to a latex backend, suggesting to use the support provided by LyX instead of allowing this privilege to any document. This is all we can do in this case to try to increase security, because we should't change users' choices. When a document with shell-escape privileges is moved to a new location or removed, it loses the privilege. So, if a new file with same name is later placed there, it doesn't inherit the privilege. To try the patch, the emblem-shellescape.svgz icon should be copied to lib/images. -- Enrico
diff --git a/src/Buffer.cpp b/src/Buffer.cpp index df7efa8f0c..3f3f706251 100644 --- a/src/Buffer.cpp +++ b/src/Buffer.cpp @@ -55,6 +55,7 @@ #include "ParagraphParameters.h" #include "ParIterator.h" #include "PDFOptions.h" +#include "Session.h" #include "SpellChecker.h" #include "sgml.h" #include "texstream.h" @@ -980,6 +981,8 @@ int Buffer::readHeader(Lexer & lex) errorList.push_back(ErrorItem(_("Document header error"), s)); } + params().shell_escape = theSession().shellescapeFiles().find(absFileName()); + params().makeDocumentClass(); return unknown_tokens; diff --git a/src/BufferParams.cpp b/src/BufferParams.cpp index f4890eaeec..d69c9f1ecf 100644 --- a/src/BufferParams.cpp +++ b/src/BufferParams.cpp @@ -38,6 +38,7 @@ #include "Lexer.h" #include "LyXRC.h" #include "OutputParams.h" +#include "Session.h" #include "Spacing.h" #include "texstream.h" #include "TexRow.h" @@ -459,6 +460,7 @@ BufferParams::BufferParams() html_css_as_file = false; display_pixel_ratio = 1.0; + shell_escape = false; output_sync = false; use_refstyle = true; use_minted = false; @@ -1441,6 +1443,21 @@ void BufferParams::writeFile(ostream & os, Buffer const * buf) const os << "\\html_latex_end " << html_latex_end << '\n'; os << pimpl_->authorlist; + + if (!shell_escape) { + std::set<std::string> & shellescape_files = + theSession().shellescapeFiles().shellescapeFiles(); + char const * ext[2] = { ",0", ",1" }; + for (int i = 0; i < 2; ++i) { + std::set<std::string>::iterator it = + shellescape_files.find(buf->absFileName() + ext[i]); + if (it != shellescape_files.end()) + shellescape_files.erase(it); + } + } else if (!theSession().shellescapeFiles().find(buf->absFileName())) { + string const name = buf->absFileName() + ",0"; + theSession().shellescapeFiles().shellescapeFiles().insert(name); + } } diff --git a/src/BufferParams.h b/src/BufferParams.h index c20601e2ac..bc5c10d194 100644 --- a/src/BufferParams.h +++ b/src/BufferParams.h @@ -535,6 +535,8 @@ public: std::string html_latex_end; /// bool html_css_as_file; + /// allow the LaTeX backend to run external programs + bool shell_escape; /// generate output usable for reverse/forward search bool output_sync; /// custom LaTeX macro from user instead our own diff --git a/src/Converter.cpp b/src/Converter.cpp index d7bdf0a7d5..61e228465c 100644 --- a/src/Converter.cpp +++ b/src/Converter.cpp @@ -279,20 +279,52 @@ 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()) + string conv_command = conv.command(); + bool const has_shell_escape = contains(conv_command, "-shell-escape") + || contains(conv_command, "-enable-write18"); + if (conv.latex() && has_shell_escape && !use_shell_escape) { + docstring const shellescape_warning = + bformat(_("<p>The following LaTeX backend has been " + "configured to allow execution of external programs " + "for any document:</p>" + "<center><p><tt>%1$s</tt></p></center>" + "<p>This is a dangerous configuration. Please, " + "consider using the support offered by LyX for " + "allowing this privilege only to documents that " + "actually need it, instead.</p>"), + from_utf8(conv_command)); + frontend::Alert::error(_("Security Warning"), + shellescape_warning , false); + } else 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>" + 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>The following LaTeX backend has been requested " + "to allow execution of 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 +334,51 @@ 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(); - 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")); - if (choice == 2) - auth_files.insert(doc_fname); + string const name = doc_fname + (use_shell_escape ? ",1" : ""); + std::set<std::string> & auth_files = use_shell_escape + ? theSession().shellescapeFiles().shellescapeFiles() + : theSession().authFiles().authFiles(); + if (auth_files.find(name) == auth_files.end()) { + choice = frontend::Alert::prompt(security_title, + security_warning2, + 0, 0, no, yes, always); + if (choice == 2) { + if (use_shell_escape) { + std::set<std::string>::iterator it = + auth_files.find(doc_fname + ",0"); + if (it != auth_files.end()) + auth_files.erase(it); + } + auth_files.insert(name); + } } 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 +513,8 @@ bool Converters::convert(Buffer const * buffer, "tmpfile.out")); } - if (!checkAuth(conv, buffer ? buffer->absFileName() : string())) + if (!checkAuth(conv, buffer ? buffer->absFileName() : string(), + buffer && buffer->params().shell_escape)) return false; if (conv.latex()) { @@ -470,6 +525,9 @@ bool Converters::convert(Buffer const * buffer, command = subst(command, token_from, ""); command = subst(command, token_latex_encoding, buffer->params().encoding().latexName()); + if (buffer->params().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 da3136a69c..8f63aba512 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 3dc2ebc9ee..75b9fbf4fa 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,46 @@ 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.substr(0, s.length() - 2)); + if (file.exists() && !file.isDirectory()) + shellescape_files_.insert(s); + else + LYXERR(Debug::INIT, "LyX: Warning: Ignore shellescape file: " << file); + } 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")); +} + + +bool ShellEscapeSection::find(string const & name) const +{ + if (shellescape_files_.find(name + ",0") != shellescape_files_.end()) + return true; + + if (shellescape_files_.find(name + ",1") != shellescape_files_.end()) + return true; + + return false; +} + + } diff --git a/src/Session.h b/src/Session.h index f471f4d28e..ad40833132 100644 --- a/src/Session.h +++ b/src/Session.h @@ -341,6 +341,30 @@ private: }; +class ShellEscapeSection : SessionSection +{ +public: + /// + explicit ShellEscapeSection() {}; + + /// + void read(std::istream & is); + + /// + void write(std::ostream & os) const; + + /// + bool find(std::string const & name) 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 +397,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 +430,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/ButtonPolicy.cpp b/src/frontends/qt4/ButtonPolicy.cpp index 1866a3c2ca..b151b065a8 100644 --- a/src/frontends/qt4/ButtonPolicy.cpp +++ b/src/frontends/qt4/ButtonPolicy.cpp @@ -398,6 +398,7 @@ void ButtonPolicy::Private::initNoRepeatedApplyReadOnly() state_machine_[INITIAL][SMI_VALID] = VALID; state_machine_[INITIAL][SMI_INVALID] = INVALID; state_machine_[INITIAL][SMI_READ_ONLY] = RO_INITIAL; + state_machine_[INITIAL][SMI_APPLY] = INITIAL; // State::VALID state_machine_[VALID][SMI_VALID] = VALID; state_machine_[VALID][SMI_READ_WRITE] = VALID; diff --git a/src/frontends/qt4/GuiDocument.cpp b/src/frontends/qt4/GuiDocument.cpp index 89cd1c944a..550628efaa 100644 --- a/src/frontends/qt4/GuiDocument.cpp +++ b/src/frontends/qt4/GuiDocument.cpp @@ -792,6 +792,8 @@ GuiDocument::GuiDocument(GuiView & lv) connect(outputModule->mathoutCB, SIGNAL(currentIndexChanged(int)), this, SLOT(change_adaptor())); + connect(outputModule->shellescapeCB, SIGNAL(stateChanged(int)), + this, SLOT(shellescapeChanged())); connect(outputModule->outputsyncCB, SIGNAL(clicked()), this, SLOT(change_adaptor())); connect(outputModule->synccustomCB, SIGNAL(editTextChanged(QString)), @@ -1537,6 +1539,15 @@ void GuiDocument::change_adaptor() } +void GuiDocument::shellescapeChanged() +{ + bool wasclean = buffer().isClean(); + slotApply(); + if (wasclean) + buffer().markClean(); +} + + void GuiDocument::includeonlyClicked(QTreeWidgetItem * item, int) { if (item == 0) @@ -3115,6 +3126,8 @@ void GuiDocument::applyView() bool const nontexfonts = fontModule->osFontsCB->isChecked(); bp_.useNonTeXFonts = nontexfonts; + bp_.shell_escape = outputModule->shellescapeCB->isChecked(); + bp_.output_sync = outputModule->outputsyncCB->isChecked(); bp_.output_sync_macro = fromqstr(outputModule->synccustomCB->currentText()); @@ -3733,6 +3746,7 @@ void GuiDocument::paramsToDialog() index = 0; outputModule->defaultFormatCO->setCurrentIndex(index); + outputModule->shellescapeCB->setChecked(bp_.shell_escape); outputModule->outputsyncCB->setChecked(bp_.output_sync); outputModule->synccustomCB->setEditText(toqstr(bp_.output_sync_macro)); diff --git a/src/frontends/qt4/GuiDocument.h b/src/frontends/qt4/GuiDocument.h index 2d0fc88822..5efde8eee3 100644 --- a/src/frontends/qt4/GuiDocument.h +++ b/src/frontends/qt4/GuiDocument.h @@ -90,6 +90,7 @@ public Q_SLOTS: private Q_SLOTS: void updateNumbering(); void change_adaptor(); + void shellescapeChanged(); void includeonlyClicked(QTreeWidgetItem * item, int); void setListingsMessage(); void listingsPackageChanged(int); diff --git a/src/frontends/qt4/GuiView.cpp b/src/frontends/qt4/GuiView.cpp index 5ff15c8e49..a3e647f7fb 100644 --- a/src/frontends/qt4/GuiView.cpp +++ b/src/frontends/qt4/GuiView.cpp @@ -577,9 +577,18 @@ GuiView::GuiView(int id) busylabel, SLOT(hide())); QFontMetrics const fm(statusBar()->fontMetrics()); - int const roheight = max(int(d.normalIconSize), fm.height()); - QSize const rosize(roheight, roheight); - QPixmap readonly = QIcon(getPixmap("images/", "emblem-readonly", "svgz,png")).pixmap(rosize); + int const iconheight = max(int(d.normalIconSize), fm.height()); + QSize const iconsize(iconheight, iconheight); + + QPixmap shellescape = QIcon(getPixmap("images/", "emblem-shellescape", "svgz,png")).pixmap(iconsize); + shell_escape_ = new QLabel(statusBar()); + shell_escape_->setPixmap(shellescape); + shell_escape_->setScaledContents(true); + shell_escape_->setAlignment(Qt::AlignCenter); + shell_escape_->hide(); + statusBar()->addPermanentWidget(shell_escape_); + + QPixmap readonly = QIcon(getPixmap("images/", "emblem-readonly", "svgz,png")).pixmap(iconsize); read_only_ = new QLabel(statusBar()); read_only_->setPixmap(readonly); read_only_->setScaledContents(true); @@ -1158,6 +1167,11 @@ void GuiView::updateWindowTitle(GuiWorkArea * wa) // Tell Qt whether the current document is changed setWindowModified(!buf.isClean()); + if (buf.params().shell_escape) + shell_escape_->show(); + else + shell_escape_->hide(); + if (buf.hasReadonlyFlag()) read_only_->show(); else diff --git a/src/frontends/qt4/GuiView.h b/src/frontends/qt4/GuiView.h index a3158a42f3..af436d4752 100644 --- a/src/frontends/qt4/GuiView.h +++ b/src/frontends/qt4/GuiView.h @@ -459,6 +459,8 @@ private: /// Request to give focus to minibuffer bool minibuffer_focus_; + /// Statusbar widget that shows shell-escape status + QLabel * shell_escape_; /// Statusbar widget that shows read-only status QLabel * read_only_; /// Statusbar widget that shows version control status diff --git a/src/frontends/qt4/GuiWorkArea.cpp b/src/frontends/qt4/GuiWorkArea.cpp index 7b46eee74f..f232003ab3 100644 --- a/src/frontends/qt4/GuiWorkArea.cpp +++ b/src/frontends/qt4/GuiWorkArea.cpp @@ -250,7 +250,7 @@ GuiWorkArea::Private::Private(GuiWorkArea * parent) cursor_visible_(false), cursor_(0), need_resize_(false), schedule_redraw_(false), preedit_lines_(1), pixel_ratio_(1.0), - completer_(new GuiCompleter(p, p)), dialog_mode_(false), + completer_(new GuiCompleter(p, p)), dialog_mode_(false), shell_escape_(false), read_only_(false), clean_(true), externally_modified_(false) { } @@ -1401,11 +1401,13 @@ void GuiWorkArea::updateWindowTitle() { Buffer const & buf = bufferView().buffer(); if (buf.fileName() != d->file_name_ + || buf.params().shell_escape != d->shell_escape_ || buf.hasReadonlyFlag() != d->read_only_ || buf.lyxvc().vcstatus() != d->vc_status_ || buf.isClean() != d->clean_ || buf.notifiesExternalModification() != d->externally_modified_) { d->file_name_ = buf.fileName(); + d->shell_escape_ = buf.params().shell_escape; d->read_only_ = buf.hasReadonlyFlag(); d->vc_status_ = buf.lyxvc().vcstatus(); d->clean_ = buf.isClean(); diff --git a/src/frontends/qt4/GuiWorkArea_Private.h b/src/frontends/qt4/GuiWorkArea_Private.h index a7eea0b1d8..f90b2c71c7 100644 --- a/src/frontends/qt4/GuiWorkArea_Private.h +++ b/src/frontends/qt4/GuiWorkArea_Private.h @@ -191,6 +191,8 @@ struct GuiWorkArea::Private /// support::FileName file_name_; /// + bool shell_escape_; + /// bool read_only_; /// docstring vc_status_; diff --git a/src/frontends/qt4/ui/OutputUi.ui b/src/frontends/qt4/ui/OutputUi.ui index 2bcf0f15cb..9e6b2a4c1f 100644 --- a/src/frontends/qt4/ui/OutputUi.ui +++ b/src/frontends/qt4/ui/OutputUi.ui @@ -80,7 +80,7 @@ </layout> </widget> </item> - <item row="4" column="0"> + <item row="5" column="0"> <widget class="QGroupBox" name="savingGB"> <property name="title"> <string>LyX Format</string> @@ -106,6 +106,16 @@ </widget> </item> <item row="1" column="0"> + <widget class="QCheckBox" name="shellescapeCB"> + <property name="toolTip"> + <string>Calls the LaTeX backend with the -shell-escape option</string> + </property> + <property name="text"> + <string>&Allow running external programs</string> + </property> + </widget> + </item> + <item row="2" column="0"> <widget class="QGroupBox" name="outputsyncCB"> <property name="toolTip"> <string>Enable forward/reverse search between editor and output (e.g., SyncTeX)</string> @@ -150,7 +160,7 @@ </layout> </widget> </item> - <item row="2" column="0"> + <item row="3" column="0"> <widget class="QGroupBox" name="xhtmlGB"> <property name="title"> <string>XHTML Output Options</string> @@ -269,7 +279,7 @@ </layout> </widget> </item> - <item row="5" column="0"> + <item row="6" column="0"> <spacer name="verticalSpacer"> <property name="orientation"> <enum>Qt::Vertical</enum>