commit e1086ed96c483fd1463fb884a92bd20653821498
Author: Pavel Sanda <[email protected]>
Date: Wed Jul 15 13:53:39 2026 +0200
Security hardening backport for LyX 2.3.x
Accumulated backport of the coordinated LyX security release, encompassing
Tier 00 variants. Folded hardening cases (per-case detail in the advisory):
00a kpsewhich filename -> shell command open/export -> exec
00b lyx2lyx invocation filename open -> exec
00c graphics filename extension -> os.system() open -> exec
00d \bibtex_command (preview + export) open/export -> exec
00e \index_command (whitelist + <> redirection) export -> exec
00g mangled graphics filename extension export -> exec
00h document basename -> conversion helpers open/import -> exec
[DiD]
00i document basename backtick in "..." export -> exec
00k \paperwidth/\paperheight -> parsecmd redirect export -> file
write
00de processing consent gate (biber/xindy/xindex) authorization guard
00h ships as defence-in-depth on 2.3: reviewed and applied, but the pre-2.4
proof-of-concept does not reproduce here (a working exploit is nonetheless
likely - cold analysis found further sinks of the same class).
The authorization gate is LyX's guard for tools that run document-embedded
code under their default command; the real fixes are upstream (biber 2.22,
xindex 1.07, coordinated TeX Live xindy update).
The gate relaxes for backported biber 2.22.
Assisted-by: Claude Opus 4.8
---
src/Buffer.cpp | 18 ++++-
src/BufferParams.cpp | 26 ++++++-
src/Converter.cpp | 32 +++++++-
src/LaTeX.cpp | 150 ++++++++++++++++++++++++++++++++++++-
src/LaTeX.h | 5 ++
src/graphics/GraphicsConverter.cpp | 9 ++-
src/graphics/PreviewLoader.cpp | 7 +-
src/support/FileName.cpp | 12 ++-
src/support/filetools.cpp | 19 ++++-
9 files changed, 262 insertions(+), 16 deletions(-)
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index ec480f7584..877af07496 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -1333,12 +1333,22 @@ Buffer::ReadStatus Buffer::convertLyXFormat(FileName
const & fn,
// Run lyx2lyx:
// $python$ "$lyx2lyx$" -t $LYX_FORMAT$ -o "$tempfile$" "$filetoread$"
+
+ // guard against command expansion in filename strings on linux,
+ // keep " on windows
+ auto sh_quote = [](string const & s) -> string {
+#ifdef _WIN32
+ return quoteName(s);
+#else
+ return '\'' + subst(s, "'", "'\\''") + '\'';
+#endif
+ };
ostringstream command;
command << os::python()
- << ' ' << quoteName(lyx2lyx.toFilesystemEncoding())
+ << ' ' << sh_quote(lyx2lyx.toFilesystemEncoding())
<< " -t " << convert<string>(LYX_FORMAT)
- << " -o " << quoteName(tmpfile.toSafeFilesystemEncoding())
- << ' ' << quoteName(fn.toSafeFilesystemEncoding());
+ << " -o " << sh_quote(tmpfile.toSafeFilesystemEncoding())
+ << ' ' << sh_quote(fn.toSafeFilesystemEncoding());
string const command_str = command.str();
LYXERR(Debug::INFO, "Running '" << command_str << '\'');
@@ -4512,6 +4522,8 @@ Buffer::ExportStatus Buffer::doExport(string const &
target, bool put_in_tempdir
theFormats().extension(backend_format));
LYXERR(Debug::FILES, "filename=" << filename);
+ // (00i-wide) makeLatexName keep-set is now shell-safe; no re-sanitize
here.
+
// Plain text backend
if (backend_format == "text") {
runparams.flavor = OutputParams::TEXT;
diff --git a/src/BufferParams.cpp b/src/BufferParams.cpp
index 51aab82aeb..adae2ee530 100644
--- a/src/BufferParams.cpp
+++ b/src/BufferParams.cpp
@@ -1021,8 +1021,18 @@ string BufferParams::readToken(Lexer & lex, string const
& token,
lcolor.setColor("boxbgcolor", color);
} else if (token == "\\paperwidth") {
lex >> paperwidth;
+ if (!paperwidth.empty() && !isValidLength(paperwidth)) {
+ lyxerr << "Rejecting non-Length \\paperwidth value: "
+ << paperwidth << endl;
+ paperwidth.clear();
+ }
} else if (token == "\\paperheight") {
lex >> paperheight;
+ if (!paperheight.empty() && !isValidLength(paperheight)) {
+ lyxerr << "Rejecting non-Length \\paperheight value: "
+ << paperheight << endl;
+ paperheight.clear();
+ }
} else if (token == "\\leftmargin") {
lex >> leftmargin;
} else if (token == "\\topmargin") {
@@ -3628,8 +3638,20 @@ string const BufferParams::getBibtexCommand(string const
cmd, bool const warn) c
string const BufferParams::bibtexCommand(bool const warn) const
{
// Return document-specific setting if available
- if (bibtex_command != "default")
- return getBibtexCommand(bibtex_command, warn);
+ if (bibtex_command != "default") {
+
+ // Block redirection on the export bibtex call.
+ // Temporary hotfix, longterm solution needs structural
+ // split between program and options.
+ static char const * const SUSPECT_CHARS = "<>\"\\\t\n";
+ if (bibtex_command.find_first_of(SUSPECT_CHARS) == string::npos)
+ return getBibtexCommand(bibtex_command, warn);
+ if (warn)
+ frontend::Alert::warning(
+ _("Requested bibliography command rejected"),
+ _("The bibliography processor command contains
prohibited characters."));
+ // fall through to the lyxrc-driven selection below
+ }
// If we have "default" in document settings, consult the prefs
// 1. Japanese (uses a specific processor)
diff --git a/src/Converter.cpp b/src/Converter.cpp
index 666e61c30b..6d754533d4 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -468,8 +468,35 @@ bool Converters::convert(Buffer const * buffer,
&& buffer->params().encoding().package() ==
Encoding::japanese;
runparams.use_indices = buffer->params().use_indices;
runparams.bibtex_command = buffer->params().bibtexCommand(true);
- runparams.index_command = (buffer->params().index_command ==
"default") ?
- string() : buffer->params().index_command;
+
+ // Accept only programs from fixed known list
+ string accepted_index_cmd;
+ if (buffer->params().index_command != "default"
+ && !buffer->params().index_command.empty()) {
+
+ // Do not allow redirection in index commands
+ bool const has_redirect =
+
buffer->params().index_command.find_first_of("<>") != string::npos;
+ if (!has_redirect) {
+ string supplied_prog;
+ split(buffer->params().index_command,
supplied_prog, ' ');
+ for (auto const & alt :
lyxrc.index_alternatives) {
+ string alt_prog;
+ split(alt, alt_prog, ' ');
+ if (!supplied_prog.empty()
+ && supplied_prog == alt_prog) {
+ accepted_index_cmd =
buffer->params().index_command;
+ break;
+ }
+ }
+ }
+ if (accepted_index_cmd.empty())
+ LYXERR0("Document-supplied index command '"
+ << buffer->params().index_command << "'
is not a recognised "
+ "index processor; falling back to
default.");
+ }
+
+ runparams.index_command = accepted_index_cmd;
runparams.document_language =
buffer->params().language->babel();
runparams.only_childbibs = !buffer->params().useBiblatex()
&& !buffer->params().useBibtopic()
@@ -810,6 +837,7 @@ bool Converters::runLaTeX(Buffer const & buffer, string
const & command,
string const name = buffer.latexName();
LaTeX latex(command, runparams, FileName(makeAbsPath(name)),
buffer.filePath(), buffer.layoutPos(),
+ buffer.absFileName(),
buffer.lastPreviewError());
TeXErrors terr;
// The connection closes itself at the end of the scope when latex is
diff --git a/src/LaTeX.cpp b/src/LaTeX.cpp
index 4be0dad4ab..3febcb1df1 100644
--- a/src/LaTeX.cpp
+++ b/src/LaTeX.cpp
@@ -30,9 +30,14 @@
#include "support/Systemcall.h"
#include "support/os.h"
+#include "Session.h"
+
+#include "frontends/alert.h"
+
#include "support/regex.h"
#include <fstream>
+#include <map>
#include <stack>
@@ -58,6 +63,10 @@ docstring runMessage(unsigned int count)
return bformat(_("Waiting for LaTeX run number %1$d"), count);
}
+bool isProcessorGated(std::string const & command);
+bool checkProcessorAuth(std::string const & doc_fname,
+ std::string const & command);
+
} // namespace
/*
@@ -94,8 +103,9 @@ bool operator!=(AuxInfo const & a, AuxInfo const & o)
LaTeX::LaTeX(string const & latex, OutputParams const & rp,
FileName const & f, string const & p, string const & lp,
- bool const clean_start)
- : cmd(latex), file(f), path(p), lpath(lp), runparams(rp), biber(false)
+ string const & dfname, bool const clean_start)
+ : cmd(latex), file(f), path(p), lpath(lp), doc_fname(dfname),
+ runparams(rp), biber(false)
{
num_errors = 0;
// lualatex can still produce a DVI with --output-format=dvi. However,
@@ -455,6 +465,12 @@ bool LaTeX::runMakeIndex(string const & f, OutputParams
const & runparams,
if (!runparams.index_command.empty())
tmp = runparams.index_command;
+ // Gate the resolved index processor `tmp` that will actually run:
+ // meant for xindy/texindy/xindex, override or default.
+ if (isProcessorGated(tmp)
+ && !checkProcessorAuth(doc_fname, tmp))
+ return false;
+
LYXERR(Debug::LATEX,
"idx file has been made, running index processor ("
<< tmp << ") on file " << f);
@@ -619,10 +635,140 @@ void LaTeX::updateBibtexDependencies(DepTable & dep,
}
+namespace {
+
+// One row per processor we can clear without gating: either a non-interpreter
+// tool that is never dangerous (safe = true, no probe), or a code-capable tool
+// at/above a version whose sinks are fixed (safe = false + version probe).
+//
+// A processor *absent* from this table is always gated (the default both for
+// code-capable tools with no acceptable version yet - xindy, xindex - and as a
+// fail-safe).
+struct RequiredProcessor {
+ char const * prog; // first-token basename to match
+ bool safe; // true = not code-capable (makeindex-class):
+ // never gate, skip the version probe
+ char const * version_arg; // argument that prints the version
+ char const * version_re; // regex capturing (major)(minor)
+ int min_major; // minimum version not requiring the gate
+ int min_minor;
+};
+
+RequiredProcessor const required_processors[] = {
+ // Non-interpreter index processors (makeindex-class): they cannot
execute
+ // document-controlled code, so never gate them and skip the probe.
+ { "makeindex", true, nullptr, nullptr, 0, 0 },
+ { "upmendex", true, nullptr, nullptr, 0, 0 },
+ // biber: code-capable; fixed upstream at 2.22
+ { "biber", false, "--version", "version:\\s*([0-9]+)\\.([0-9]+)", 2, 22
},
+};
+
+// False only when >= required version.
+// True for an unknown tool, an unparseable version, or a failed probe a
+// Caches one `--version` probe per processor per session.
+bool isProcessorGated(string const & command)
+{
+ string prog;
+ split(command, prog, ' '); // first whitespace token only
+ prog = onlyFileName(prog); // strip any directory part
+ if (prog.empty())
+ return true;
+
+ static map<string, bool> cache;
+ map<string, bool>::const_iterator const it = cache.find(prog);
+ if (it != cache.end())
+ return it->second;
+
+ bool gated = true; // fail-safe default
+ for (RequiredProcessor const & p : required_processors) {
+ if (prog != p.prog)
+ continue;
+ if (p.safe) { // non-interpreter: never gate, no
probe
+ gated = false;
+ break;
+ }
+ //safe because prog was matched against the table
+ cmd_ret const r =
+ runCommand(quoteName(prog) + ' ' + p.version_arg);
+ smatch m;
+ regex const re(p.version_re);
+ if (r.first == 0 && regex_search(r.second, m, re)) {
+ int const maj = convert<int>(m.str(1));
+ int const min = convert<int>(m.str(2));
+ gated = maj < p.min_major
+ || (maj == p.min_major && min < p.min_minor);
+ }
+ break; // matched the table row
+ }
+ cache[prog] = gated;
+ return gated;
+}
+
+// Per-document trust gate; this only handles consent.
+//
+// Deliberately reuses Converters::checkAuth's machinery so the trust
+// decision is shared: the same per-document authorization set
+// (theSession().authFiles()), the same global prompt switch
+// (lyxrc.use_converter_needauth), and the same persisted "Always run for this
+// document". A document trusted for a needauth converter is therefore also
+// trusted here, and vice versa - one "do you trust this document?" decision.
+//
+// Unlike checkAuth it does NOT honour use_converter_needauth_forbidden: that
+// pref defaults to "forbid", which is correct for the rare hand-flagged
+// needauth converters but would block biber on *every* biblatex document and
+// xindy/xindex on every indexed one. Gating here is consent, not a
+// hard-deny master switch.
+//
+// Returns true if the processor may run.
+bool checkProcessorAuth(string const & doc_fname, string const & command)
+{
+ if (!lyxrc.use_converter_needauth)
+ return true;
+
+ docstring const title =
+ _("A LaTeX backend requires your authorization");
+ docstring const warning = bformat(
+ _("<p>The following LaTeX backend has been requested "
+ "to allow execution of external programs:</p>"
+ "<center><p>%1$s</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("<tt>" + command + "</tt>"))
+ + _("<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>");
+
+ // No document identity (preview, clone, import): cannot persist a
+ // per-document decision, so prompt without the "Always" option.
+ if (doc_fname.empty())
+ return frontend::Alert::prompt(title, warning, 0, 0,
+ _("Do ¬ allow"), _("A&llow")) != 0;
+
+ if (theSession().authFiles().find(doc_fname))
+ return true;
+
+ int const choice = frontend::Alert::prompt(title, warning, 0, 0,
+ _("Do ¬ allow"), _("A&llow"),
+ _("&Always allow for this document"));
+ if (choice == 2)
+ theSession().authFiles().insert(doc_fname);
+ return choice != 0;
+}
+
+} // namespace
+
+
bool LaTeX::runBibTeX(vector<AuxInfo> const & bibtex_info,
OutputParams const & runparams)
{
bool result = false;
+
+ // Old biber is not safe. Plain bibtex is safe.
+ if (biber && isProcessorGated(runparams.bibtex_command)
+ && !checkProcessorAuth(doc_fname, runparams.bibtex_command))
+ return false;
+
for (vector<AuxInfo>::const_iterator it = bibtex_info.begin();
it != bibtex_info.end(); ++it) {
if (!biber && it->databases.empty())
diff --git a/src/LaTeX.h b/src/LaTeX.h
index 0b46c607af..012c5a0759 100644
--- a/src/LaTeX.h
+++ b/src/LaTeX.h
@@ -162,6 +162,7 @@ public:
support::FileName const & file,
std::string const & path = empty_string(),
std::string const & lpath = empty_string(),
+ std::string const & doc_fname = empty_string(),
bool const clean_start = false);
/// runs LaTeX several times
@@ -231,6 +232,10 @@ private:
/// Extra path, possibly relative to the document directory path.
std::string lpath;
+ /// Absolute name for unique cache record in the trust gate.
+ /// Shared with Converters::checkAuth.
+ std::string doc_fname;
+
/// used by scanLogFile
int num_errors;
diff --git a/src/graphics/GraphicsConverter.cpp
b/src/graphics/GraphicsConverter.cpp
index 598e108b73..b7a59b97ac 100644
--- a/src/graphics/GraphicsConverter.cpp
+++ b/src/graphics/GraphicsConverter.cpp
@@ -30,6 +30,8 @@
#include "support/TempFile.h"
#include <sstream>
+#include <algorithm>
+#include <cctype>
#include <fstream>
using namespace std;
@@ -302,7 +304,12 @@ static void build_script(string const & doc_fname,
theConverters().getPath(from_format, to_format);
// Create a temporary base file-name for all intermediate steps.
- string const from_ext = getExtension(from_file);
+ // The extension string is user-controlled. Avoid metacharacters
+ // to prevent havoc down the pipeline.
+ string from_ext = getExtension(from_file);
+ from_ext.erase(remove_if(from_ext.begin(), from_ext.end(),
+ [](unsigned char c){ return !(isalnum(c) || c == '_' || c ==
'-'); }),
+ from_ext.end());
TempFile tempfile(addExtension("gconvertXXXXXX", from_ext));
tempfile.setAutoRemove(false);
string outfile = tempfile.name().toFilesystemEncoding();
diff --git a/src/graphics/PreviewLoader.cpp b/src/graphics/PreviewLoader.cpp
index 0d65ccf507..2714ed9254 100644
--- a/src/graphics/PreviewLoader.cpp
+++ b/src/graphics/PreviewLoader.cpp
@@ -718,7 +718,12 @@ void PreviewLoader::Impl::startLoading(bool wait)
}
cs << latexparam;
- cs << " --bibtex=" << quoteName(buffer_.params().bibtexCommand());
+
+ // --bibtex= allows document-controlled arbitrary code
+ // execution in lyxpreview_tools.py. Tradeoff when disabling
+ // it is unresolved citations inside math/ERT preview.
+ //cs << " --bibtex=" << quoteName(buffer_.params().bibtexCommand());
+
if (buffer_.params().bufferFormat() == "lilypond-book")
cs << " --lilypond";
diff --git a/src/support/FileName.cpp b/src/support/FileName.cpp
index 482bb93e21..8c3b2e8dfc 100644
--- a/src/support/FileName.cpp
+++ b/src/support/FileName.cpp
@@ -972,14 +972,20 @@ string DocFileName::mangledFileName(string const & dir)
const
// are forbidden: '/', '.', ' ', and ':'.
// On windows it is not possible to create files with '<', '>' or '?'
// in the name.
+ // We forbid ';', '=' as they coudl become active in shell.
static string const keep = "abcdefghijklmnopqrstuvwxyz"
"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
- "+-0123456789;=";
+ "+-0123456789";
string::size_type pos = 0;
while ((pos = mname.find_first_not_of(keep, pos)) != string::npos)
mname[pos++] = '_';
- // Add the extension back on
- mname = support::changeExtension(mname, getExtension(name));
+ // Add the extension back on, but sanitize from metachars,
+ // it's user-controlled string.
+ string ext = getExtension(name);
+ pos = 0;
+ while ((pos = ext.find_first_not_of(keep, pos)) != string::npos)
+ ext[pos++] = '_';
+ mname = support::changeExtension(mname, ext);
// Prepend a counter to the filename. This is necessary to make
// the mangled name unique.
diff --git a/src/support/filetools.cpp b/src/support/filetools.cpp
index f2697f784b..073d0511f8 100644
--- a/src/support/filetools.cpp
+++ b/src/support/filetools.cpp
@@ -206,7 +206,7 @@ FileName const makeLatexName(FileName const & file)
// a non-latin world out there...
string const keep = "abcdefghijklmnopqrstuvwxyz"
"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
- "@!'()*+,-./0123456789:;<=>?[]`|";
+ "0123456789+-._,@";
string::size_type pos = 0;
while ((pos = name.find_first_not_of(keep, pos)) != string::npos)
@@ -1164,7 +1164,22 @@ FileName const findtexfile(string const & fil, string
const & /*format*/,
// tfm - TFMFONTS, TEXFONTS
// This means that to use kpsewhich in the best possible way we
// should help it by setting additional path in the approp. envir.var.
- string const kpsecmd = "kpsewhich " + fil;
+
+ if (fil.empty())
+ return FileName();
+
+ // Wrap fil in the shell's quoting form that disables the relevant
+ // metacharacter set.
+#ifdef _WIN32
+ // Reject '"' in filename, can't be backslashed & forbidden by NTFS
anyway
+ if (fil.find('"') != string::npos)
+ return FileName();
+ // disable metacharacters
+ string const kpsecmd = "kpsewhich -- \"" + fil + "\"";
+#else
+ // disable metacharacters & escape existing '
+ string const kpsecmd = "kpsewhich -- '" + subst(fil, "'", "'\\''") +
"'";
+#endif
cmd_ret const c = runCommand(kpsecmd);
--
lyx-cvs mailing list
[email protected]
https://lists.lyx.org/mailman/listinfo/lyx-cvs
