commit 10e012639c9e53a8fdf7740e083aed99c0c6fadd
Author: Pavel Sanda <[email protected]>
Date: Wed Jul 15 13:51:00 2026 +0200
Security hardening backport for LyX 2.4.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
00i document basename backtick in "..." export -> exec
00k \paperwidth/\paperheight -> parsecmd redirect export -> file
write
00de processing consent gate (biber/xindy/xindex) authorization guard
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 biber reporting version >= 2.22.
Assisted-by: Claude Opus 4.8
---
src/Buffer.cpp | 19 ++++-
src/BufferParams.cpp | 26 ++++++-
src/Converter.cpp | 31 +++++++-
src/LaTeX.cpp | 152 ++++++++++++++++++++++++++++++++++++-
src/LaTeX.h | 5 ++
src/graphics/GraphicsConverter.cpp | 9 ++-
src/graphics/PreviewLoader.cpp | 7 +-
src/support/FileName.cpp | 12 +--
src/support/filetools.cpp | 22 +++++-
9 files changed, 263 insertions(+), 20 deletions(-)
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index 73de572eb0..bca4a9beef 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -1398,12 +1398,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 << '\'');
@@ -4595,6 +4605,9 @@ Buffer::ExportStatus Buffer::doExport(string const &
target, bool put_in_tempdir
filename = addName(temppath(), filename);
filename = changeExtension(filename,
theFormats().extension(backend_format));
+
+ // (00i-wide) makeLatexName keep-set is now shell-safe; no re-sanitize
here.
+
LYXERR(Debug::FILES, "filename=" << filename);
// Plain text backend
diff --git a/src/BufferParams.cpp b/src/BufferParams.cpp
index dcd539dfd1..04ae90c97e 100644
--- a/src/BufferParams.cpp
+++ b/src/BufferParams.cpp
@@ -1114,8 +1114,18 @@ string BufferParams::readToken(Lexer & lex, string const
& token,
isboxbgcolor = true;
} 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") {
@@ -3828,8 +3838,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 41bd8ce1a0..3c4130ebf1 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -497,8 +497,34 @@ Converters::RetVal Converters::convert(Buffer const *
buffer,
&& bp.encoding().package() == Encoding::japanese;
runparams.use_indices = bp.use_indices;
runparams.bibtex_command = bp.bibtexCommand(true);
- runparams.index_command = (bp.index_command == "default") ?
- string() : bp.index_command;
+
+ // Accept only programs from fixed known list
+ string accepted_index_cmd;
+ if (bp.index_command != "default" && !bp.index_command.empty())
{
+
+ // Do not allow redirection in index commands
+ bool const has_redirect =
+ bp.index_command.find_first_of("<>") !=
string::npos;
+ if (!has_redirect) {
+ string supplied_prog;
+ split(bp.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 =
bp.index_command;
+ break;
+ }
+ }
+ }
+ if (accepted_index_cmd.empty())
+ LYXERR0("Document-supplied index command '"
+ << bp.index_command << "' is not a
recognised "
+ "index processor; falling back to
default.");
+ }
+
+ runparams.index_command = accepted_index_cmd;
runparams.document_language = bp.language->lang();
// Some macros rely on font encoding
runparams.main_fontenc = bp.main_font_encoding();
@@ -901,6 +927,7 @@ Converters::RetVal Converters::runLaTeX(Buffer const &
buffer, string const & co
string const name = buffer.latexName();
LaTeX latex(command, runparams, makeAbsPath(name),
buffer.filePath(), buffer.layoutPos(),
+ buffer.absFileName(),
buffer.isClone(), buffer.freshStartRequired());
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 3225867ac2..9ff5bf8a02 100644
--- a/src/LaTeX.cpp
+++ b/src/LaTeX.cpp
@@ -26,6 +26,9 @@
#include "Encoding.h"
#include "Language.h"
#include "LaTeXFeatures.h"
+#include "Session.h"
+
+#include "frontends/alert.h"
#include "support/debug.h"
#include "support/docstring.h"
@@ -38,6 +41,7 @@
#include "support/os.h"
#include <fstream>
+#include <map>
#include <regex>
#include <stack>
@@ -64,6 +68,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
/*
@@ -108,10 +116,11 @@ 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,
+ FileName const & f, string const & p, string const & lp,
+ string const & dfname,
bool allow_cancellation, bool const clean_start)
- : cmd(latex), file(f), path(p), lpath(lp), runparams(rp), biber(false),
- allow_cancel(allow_cancellation)
+ : cmd(latex), file(f), path(p), lpath(lp), doc_fname(dfname),
+ runparams(rp), biber(false), allow_cancel(allow_cancellation)
{
num_errors = 0;
// lualatex can still produce a DVI with --output-format=dvi. However,
@@ -548,6 +557,12 @@ int LaTeX::runMakeIndex(string const & f, OutputParams
const & rp,
if (!rp.index_command.empty())
tmp = rp.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 Systemcall::KILLED;
+
Language const * doc_lang = languages.getLanguage(rp.document_language);
if (contains(tmp, "$$x")) {
@@ -753,11 +768,142 @@ 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.valid && regex_search(r.result, 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 & rp, int & exit_code)
{
bool result = false;
exit_code = 0;
+
+ // Old biber is not safe. Plain bibtex is safe.
+ if (biber && isProcessorGated(rp.bibtex_command)
+ && !checkProcessorAuth(doc_fname, rp.bibtex_command)) {
+ exit_code = Systemcall::KILLED;
+ 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 569114d369..e973300d12 100644
--- a/src/LaTeX.h
+++ b/src/LaTeX.h
@@ -178,6 +178,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 allow_cancellation = false,
bool const clean_start = false);
@@ -251,6 +252,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 34d37a4066..31e62e89c7 100644
--- a/src/graphics/GraphicsConverter.cpp
+++ b/src/graphics/GraphicsConverter.cpp
@@ -26,6 +26,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 be76444df5..a8b9d229cf 100644
--- a/src/graphics/PreviewLoader.cpp
+++ b/src/graphics/PreviewLoader.cpp
@@ -686,7 +686,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 864f38770c..7549a92283 100644
--- a/src/support/FileName.cpp
+++ b/src/support/FileName.cpp
@@ -996,11 +996,11 @@ string DocFileName::mangledFileName(string const & dir,
bool encrypt_path) const
// xHTML route
// we use hash instead of counter to get stable filenames in export
directory
if (encrypt_path) {
- // sanitization probably not neccessary for xhtml, but won't
harm
string sanfn = support::changeExtension(onlyFileName(),
string());
sanfn = sanitizeFileName(sanfn);
- // Add the extension back on
- sanfn = support::changeExtension(sanfn,
getExtension(onlyFileName()));
+ // extension is user-controlled string, suppress metacharacters
+ sanfn = support::changeExtension(sanfn,
+ sanitizeFileName(getExtension(onlyFileName())));
//various filesystems have filename limit around 2^8
if (sanfn.length() > 230)
@@ -1020,8 +1020,10 @@ string DocFileName::mangledFileName(string const & dir,
bool encrypt_path) const
mname = support::changeExtension(name, string());
// The mangled name must be a valid LaTeX name.
mname = sanitizeFileName(mname);
- // 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
+ mname = support::changeExtension(mname,
+ sanitizeFileName(getExtension(name)));
// Prepend a counter to the filename. This is necessary to make
// the mangled name unique, see truncation below.
diff --git a/src/support/filetools.cpp b/src/support/filetools.cpp
index 954e68d04b..7b6d189218 100644
--- a/src/support/filetools.cpp
+++ b/src/support/filetools.cpp
@@ -209,7 +209,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)
@@ -1230,7 +1230,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);
@@ -1366,9 +1381,10 @@ std::string sanitizeFileName(const std::string & str)
// are forbidden: '/', '.', ' ', and ':'.
// On windows it is not possible to create files with '<', '>' or '?'
// in the name.
+ // We forbid ';', '=' as they could become active in shell.
static std::string const keep = "abcdefghijklmnopqrstuvwxyz"
"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
- "+-0123456789;=";
+ "+-0123456789";
std::string name = str;
string::size_type pos = 0;
--
lyx-cvs mailing list
[email protected]
https://lists.lyx.org/mailman/listinfo/lyx-cvs
