last patch attached.
T.
On 06/11/2016 20:56, LyX Ticket Tracker wrote:
#10481: Hardening LyX against potential misuse
-------------------------+--------------------------
Reporter: t.cucinotta | Owner: lasgouttes
Type: enhancement | Status: new
Priority: highest | Milestone:
Component: general | Version: unspecified
Severity: major | Resolution:
Keywords: |
-------------------------+--------------------------
Comment (by t.cucinotta):
Just fixed minor things and the wording in the attached .2.patch file,
however there's the major issue that in GraphicsConverter there's no info
whatsoever about buffer name, so it won't be possible to have the "yes,
don't ask for the doc" option in that case...., that case being display
on-screen, which is probably the most common/needed (the one that works ok
is when you export to PDF or other formats).
Tried to add the missing context in a separate patch, but got to nothing
but ... segfaults!
commit c5c02d37
Author: Tommaso Cucinotta <tomm...@lyx.org>
Date: Sat Nov 5 01:00:44 2016 +0100
Add needauth option to converters that need explicit user authorization before being run.
Addressing #10481.
Converters marked with the new "needauth" option won't be run unless
the user gives explicit authorization, which is asked on-demand when
the converter is about to be run (question is not asked if the file is
cached and the converter is not needed).
The user prompt has a 3rd button (yes to all for document) so that
he/she's not prompted again for the same converter over the same document
(identified through buffer->filePath()).
diff --git a/lib/configure.py b/lib/configure.py
index 09d053e9..705aed6c 100644
--- a/lib/configure.py
+++ b/lib/configure.py
@@ -749,8 +749,8 @@ def checkConverterEntries():
rc_entry = [r'''\converter latex lyx "%% -f $$i $$o" ""
\converter latexclipboard lyx "%% -fixedenc utf8 -f $$i $$o" ""
\converter literate lyx "%% -n -m noweb -f $$i $$o" ""
-\converter sweave lyx "%% -n -m sweave -f $$i $$o" ""
-\converter knitr lyx "%% -n -m knitr -f $$i $$o" ""'''], not_found = 'tex2lyx')
+\converter sweave lyx "%% -n -m sweave -f $$i $$o" "needauth"
+\converter knitr lyx "%% -n -m knitr -f $$i $$o" "needauth"'''], not_found = 'tex2lyx')
if path == '':
logger.warning("Failed to find tex2lyx on your system.")
@@ -763,24 +763,24 @@ def checkConverterEntries():
\converter literate dviluatex "%%" ""'''])
#
checkProg('a Sweave -> LaTeX converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxsweave.R $$p$$i $$p$$o $$e $$r'],
- rc_entry = [r'''\converter sweave latex "%%" ""
-\converter sweave pdflatex "%%" ""
-\converter sweave xetex "%%" ""
-\converter sweave luatex "%%" ""
-\converter sweave dviluatex "%%" ""'''])
+ rc_entry = [r'''\converter sweave latex "%%" "needauth"
+\converter sweave pdflatex "%%" "needauth"
+\converter sweave xetex "%%" "needauth"
+\converter sweave luatex "%%" "needauth"
+\converter sweave dviluatex "%%" "needauth"'''])
#
checkProg('a knitr -> LaTeX converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxknitr.R $$p$$i $$p$$o $$e $$r'],
- rc_entry = [r'''\converter knitr latex "%%" ""
-\converter knitr pdflatex "%%" ""
-\converter knitr xetex "%%" ""
-\converter knitr luatex "%%" ""
-\converter knitr dviluatex "%%" ""'''])
+ rc_entry = [r'''\converter knitr latex "%%" "needauth"
+\converter knitr pdflatex "%%" "needauth"
+\converter knitr xetex "%%" "needauth"
+\converter knitr luatex "%%" "needauth"
+\converter knitr dviluatex "%%" "needauth"'''])
#
checkProg('a Sweave -> R/S code converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxstangle.R $$i $$e $$r'],
- rc_entry = [ r'\converter sweave r "%%" ""' ])
+ rc_entry = [ r'\converter sweave r "%%" "needauth"' ])
#
checkProg('a knitr -> R/S code converter', ['Rscript --verbose --no-save --no-restore $$s/scripts/lyxknitr.R $$p$$i $$p$$o $$e $$r tangle'],
- rc_entry = [ r'\converter knitr r "%%" ""' ])
+ rc_entry = [ r'\converter knitr r "%%" "needauth"' ])
#
checkProg('an HTML -> LaTeX converter', ['html2latex $$i', 'gnuhtml2latex',
'htmltolatex -input $$i -output $$o', 'htmltolatex.jar -input $$i -output $$o'],
diff --git a/src/Converter.cpp b/src/Converter.cpp
index 58e486e6..02631ca4 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -100,7 +100,7 @@ Converter::Converter(string const & f, string const & t,
string const & c, string const & l)
: from_(f), to_(t), command_(c), flags_(l),
From_(0), To_(0), latex_(false), xml_(false),
- need_aux_(false), nice_(false)
+ need_aux_(false), nice_(false), need_auth_(false)
{}
@@ -128,6 +128,8 @@ void Converter::readFlags()
parselog_ = flag_value;
else if (flag_name == "nice")
nice_ = true;
+ else if (flag_name == "needauth")
+ need_auth_ = true;
}
if (!result_dir_.empty() && result_file_.empty())
result_file_ = "index." + formats.extension(to_);
@@ -402,6 +404,29 @@ bool Converters::convert(Buffer const * buffer,
"tmpfile.out"));
}
+ if (conv.need_auth()) {
+ static const char securityWarning[] = "LyX is about to run converter '%1$s' which is launching an external program that normally acts as a picture/format converter. However, this external program is known to be able to execute arbitrary actions on the system, including dangerous ones such as deleting files, if instructed to do so by a maliciously crafted .lyx document.\n Would you like to proceed?\nAnswer Yes only if you trust the source/sender of the .lyx document!";
+ int choice;
+ if (buffer) {
+ std::string const & fname = buffer->filePath();
+ if (auth_files.find(fname) == auth_files.end()) {
+ choice = frontend::Alert::prompt(
+ _("Launch of external converter needs user authorization"), bformat(_(securityWarning), from_utf8(conv.command())),
+ 0, 0, _("&No"), _("&Yes"), _("Yes, &and do not ask again for document"));
+ if (choice == 2)
+ auth_files.insert(fname);
+ } else {
+ choice = 1;
+ }
+ } else {
+ choice = frontend::Alert::prompt(
+ _("Need user authorization to launch external converter"), bformat(_(securityWarning), from_utf8(conv.command())),
+ 0, 0, _("&No"), _("&Yes"));
+ }
+ if (choice == 0)
+ return false;
+ }
+
if (conv.latex()) {
run_latex = true;
string command = conv.command();
diff --git a/src/Converter.h b/src/Converter.h
index c9c44238..9bde8714 100644
--- a/src/Converter.h
+++ b/src/Converter.h
@@ -70,6 +70,8 @@ public:
///
bool need_aux() const { return need_aux_; }
///
+ bool need_auth() const { return need_auth_; }
+ ///
bool nice() const { return nice_; }
///
std::string const result_dir() const { return result_dir_; }
@@ -77,6 +79,7 @@ public:
std::string const result_file() const { return result_file_; }
///
std::string const parselog() const { return parselog_; }
+
private:
///
trivstring from_;
@@ -101,6 +104,8 @@ private:
bool need_aux_;
/// we need a "nice" file from the backend, c.f. OutputParams.nice.
bool nice_;
+ /// Use of this converter needs explicit user authorization
+ bool need_auth_;
/// If the converter put the result in a directory, then result_dir
/// is the name of the directory
trivstring result_dir_;
@@ -181,6 +186,7 @@ public:
const_iterator end() const { return converterlist_.end(); }
///
void buildGraph();
+
private:
///
FormatList const
@@ -210,6 +216,8 @@ private:
bool copy);
///
Graph G_;
+ /// set of files authorized for external conversion
+ std::set<std::string> auth_files;
};
/// The global instance.
diff --git a/src/graphics/GraphicsConverter.cpp b/src/graphics/GraphicsConverter.cpp
index 67b1580f..6017073f 100644
--- a/src/graphics/GraphicsConverter.cpp
+++ b/src/graphics/GraphicsConverter.cpp
@@ -15,6 +15,7 @@
#include "Converter.h"
#include "Format.h"
+#include "frontends/alert.h"
#include "support/lassert.h"
#include "support/convert.h"
#include "support/debug.h"
@@ -372,6 +373,15 @@ static void build_script(string const & from_file,
outfile = tempfile.name().toFilesystemEncoding();
}
+ if (conv.need_auth()) {
+ static const char securityWarning[] = "LyX is about to run converter '%1$s' which is launching an external program that normally acts as a picture/format converter. However, this external program is known to be able to execute arbitrary actions on the system, including dangerous ones such as deleting files, if instructed to do so by a maliciously crafted .lyx document.\n Would you like to proceed?\nAnswer Yes only if you trust the source/sender of the .lyx document!";
+ docstring const & question = bformat(from_utf8(securityWarning), from_utf8(conv.command()));
+ int choice = frontend::Alert::prompt(
+ from_utf8("Need user authorization to launch external converter"), question, 0, 0, from_utf8("&No"), from_utf8("&Yes"));
+ if (choice == 0)
+ return;
+ }
+
// Store these names in the python script
script << "infile = "
<< quoteName(infile, quote_python) << "\n"
