On Mon, Sep 28, 2026 at 10:14:16PM +0200, Pavel Sanda wrote: > I'll keep posting next pacthes for various issues, but should not
The next case handles git/svn invocation argument smuggling. Pavel
>From 69643ebb7f4a9d22eee2253f40001bfcd07f510b Mon Sep 17 00:00:00 2001 From: Pavel Sanda <[email protected]> Date: Tue, 6 Oct 2026 09:07:50 +0200 Subject: [PATCH] VCS: block argument smuggling Insert " -- " between every svn/git subcommand and the filename arguments that may originate from attacker-controlled paths. Excluded: * git describe - no filename argument * git show <rev>:./<file> - the argv element starts with the revision, not '-'; '--' is not supported in this syntax * CVS/RCS subcommands - older tools without reliable '--' support Tier 01 DiD / vcs-args: -- separator for VCS filename arguments Assisted-by: Claude Opus 5 --- src/VCBackend.cpp | 60 +++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/VCBackend.cpp b/src/VCBackend.cpp index 751529c9be..d38ff6d542 100644 --- a/src/VCBackend.cpp +++ b/src/VCBackend.cpp @@ -1173,7 +1173,7 @@ bool SVN::findFile(FileName const & file) // Now we check the status of the file. string const fname = onlyFileName(file.absFileName()); LYXERR(Debug::LYXVC, "LyXVC: Checking if file is under svn control for `" << fname << '\''); - bool found = 0 == doVCCommandCall("svn info " + quoteName(fname), + bool found = 0 == doVCCommandCall("svn info -- " + quoteName(fname), file.onlyPath()); LYXERR(Debug::LYXVC, "SVN control: " << (found ? "enabled" : "disabled")); return found; @@ -1200,7 +1200,7 @@ bool SVN::checkLockMode() } LYXERR(Debug::LYXVC, "Detecting locking mode..."); - if (doVCCommandCall("svn proplist " + quoteName(onlyFileName(owner_->absFileName())) + if (doVCCommandCall("svn proplist -- " + quoteName(onlyFileName(owner_->absFileName())) + " > " + quoteName(tmpf.toFilesystemEncoding()), FileName(owner_->filePath()))) return false; @@ -1235,14 +1235,14 @@ bool SVN::retrieve(FileName const & file) { LYXERR(Debug::LYXVC, "LyXVC::SVN: retrieve.\n\t" << file); // The caller ensures that file does not exist, so no need to check that. - return doVCCommandCall("svn update -q --non-interactive " + quoteName(file.onlyFileName()), + return doVCCommandCall("svn update -q --non-interactive -- " + quoteName(file.onlyFileName()), file.onlyPath()) == 0; } void SVN::registrer(string const & /*msg*/) { - doVCCommand("svn add -q --parents " + quoteName(onlyFileName(owner_->absFileName())), + doVCCommand("svn add -q --parents -- " + quoteName(onlyFileName(owner_->absFileName())), FileName(owner_->filePath())); } @@ -1260,10 +1260,10 @@ string SVN::rename(support::FileName const & newFile, string const & msg) // confusing to the user to have two uncommitted files. FileName path(owner_->filePath()); string relFile(to_utf8(newFile.relPath(path.absFileName()))); - string cmd("svn move -q " + quoteName(onlyFileName(owner_->absFileName())) + + string cmd("svn move -q -- " + quoteName(onlyFileName(owner_->absFileName())) + ' ' + quoteName(relFile)); if (doVCCommand(cmd, path)) { - cmd = "svn revert -q " + + cmd = "svn revert -q -- " + quoteName(onlyFileName(owner_->absFileName())) + ' ' + quoteName(relFile); doVCCommand(cmd, path); @@ -1276,7 +1276,7 @@ string SVN::rename(support::FileName const & newFile, string const & msg) f.push_back(newFile); string log; if (checkIn(f, msg, log) != LyXVC::VCSuccess) { - cmd = "svn revert -q " + + cmd = "svn revert -q -- " + quoteName(onlyFileName(owner_->absFileName())) + ' ' + quoteName(relFile); doVCCommand(cmd, path); @@ -1301,7 +1301,7 @@ string SVN::copy(support::FileName const & newFile, string const & msg) // confusing to the user to have an uncommitted file. FileName path(owner_->filePath()); string relFile(to_utf8(newFile.relPath(path.absFileName()))); - string cmd("svn copy -q " + quoteName(onlyFileName(owner_->absFileName())) + + string cmd("svn copy -q -- " + quoteName(onlyFileName(owner_->absFileName())) + ' ' + quoteName(relFile)); if (doVCCommand(cmd, path)) return string(); @@ -1380,7 +1380,7 @@ bool SVN::isCheckInWithConfirmation() return true; } - doVCCommandCall("svn diff " + quoteName(owner_->absFileName()) + doVCCommandCall("svn diff -- " + quoteName(owner_->absFileName()) + " > " + quoteName(tmpf.toFilesystemEncoding()), FileName(owner_->filePath())); @@ -1426,7 +1426,7 @@ bool SVN::fileLock(bool lock, FileName const & tmpf, string &status) if (!locked_mode_ || (isLocked() == lock)) return true; - string const arg = lock ? "lock " : "unlock "; + string const arg = lock ? "lock -- " : "unlock -- "; doVCCommand("svn "+ arg + quoteName(onlyFileName(owner_->absFileName())) + " > " + quoteName(tmpf.toFilesystemEncoding()), FileName(owner_->filePath())); @@ -1466,7 +1466,7 @@ string SVN::checkOut() return N_("Error: Could not generate logfile."); } - doVCCommand("svn update --non-interactive " + quoteName(onlyFileName(owner_->absFileName())) + doVCCommand("svn update --non-interactive -- " + quoteName(onlyFileName(owner_->absFileName())) + " > " + quoteName(tmpf.toFilesystemEncoding()), FileName(owner_->filePath())); @@ -1503,7 +1503,7 @@ string SVN::repoUpdate() return N_("Error: Could not generate logfile."); } - doVCCommand("svn diff " + quoteName(owner_->filePath()) + doVCCommand("svn diff -- " + quoteName(owner_->filePath()) + " > " + quoteName(tmpf.toFilesystemEncoding()), FileName(owner_->filePath())); docstring res = tmpf.fileContents("UTF-8"); @@ -1532,7 +1532,7 @@ string SVN::repoUpdate() // + " > " + quoteName(tmpf.toFilesystemEncoding()), // FileName(owner_->filePath())); // res = "Revert log:\n" + tmpf.fileContents("UTF-8"); - doVCCommand("svn update --accept mine-full " + quoteName(owner_->filePath()) + doVCCommand("svn update --accept mine-full -- " + quoteName(owner_->filePath()) + " > " + quoteName(tmpf.toFilesystemEncoding()), FileName(owner_->filePath())); res += "Update log:\n" + tmpf.fileContents("UTF-8"); @@ -1557,7 +1557,7 @@ string SVN::lockingToggle() return N_("Error: Could not generate logfile."); } - int ret = doVCCommand("svn proplist " + quoteName(onlyFileName(owner_->absFileName())) + int ret = doVCCommand("svn proplist -- " + quoteName(onlyFileName(owner_->absFileName())) + " > " + quoteName(tmpf.toFilesystemEncoding()), FileName(owner_->filePath())); if (ret) @@ -1567,12 +1567,12 @@ string SVN::lockingToggle() string res = scanLogFile(tmpf, log); bool locking = contains(res, "svn:needs-lock"); if (!locking) - ret = doVCCommand("svn propset svn:needs-lock ON " + ret = doVCCommand("svn propset svn:needs-lock ON -- " + quoteName(onlyFileName(owner_->absFileName())) + " > " + quoteName(tmpf.toFilesystemEncoding()), FileName(owner_->filePath())); else - ret = doVCCommand("svn propdel svn:needs-lock " + ret = doVCCommand("svn propdel svn:needs-lock -- " + quoteName(onlyFileName(owner_->absFileName())) + " > " + quoteName(tmpf.toFilesystemEncoding()), FileName(owner_->filePath())); @@ -1601,7 +1601,7 @@ bool SVN::revert() // gets the updated version from the repository. string const fil = quoteName(onlyFileName(owner_->absFileName())); - if (doVCCommand("svn revert -q " + fil, + if (doVCCommand("svn revert -q -- " + fil, FileName(owner_->filePath()))) return false; owner_->markClean(); @@ -1676,7 +1676,7 @@ bool SVN::getFileRevisionInfo() return false; } - doVCCommand("svn info --xml " + quoteName(onlyFileName(owner_->absFileName())) + doVCCommand("svn info --xml -- " + quoteName(onlyFileName(owner_->absFileName())) + " > " + quoteName(tmpf.toFilesystemEncoding()), FileName(owner_->filePath())); @@ -1748,7 +1748,7 @@ bool SVN::getTreeRevisionInfo() void SVN::getLog(FileName const & tmpf) { - doVCCommand("svn log " + quoteName(onlyFileName(owner_->absFileName())) + doVCCommand("svn log -- " + quoteName(onlyFileName(owner_->absFileName())) + " > " + quoteName(tmpf.toFilesystemEncoding()), FileName(owner_->filePath())); } @@ -1781,7 +1781,7 @@ bool SVN::prepareFileRevision(string const & revis, string & f) return false; } - doVCCommand("svn cat -r " + revname + ' ' + doVCCommand("svn cat -r " + revname + " -- " + quoteName(onlyFileName(owner_->absFileName())) + " > " + quoteName(tmpf.toFilesystemEncoding()), FileName(owner_->filePath())); @@ -1840,7 +1840,7 @@ bool GIT::findFile(FileName const & file) string const fname = onlyFileName(file.absFileName()); LYXERR(Debug::LYXVC, "LyXVC: Checking if file is under git control for `" << fname << '\''); - doVCCommandCall("git ls-files " + + doVCCommandCall("git ls-files -- " + quoteName(fname) + " > " + quoteName(tmpf.toFilesystemEncoding()), file.onlyPath()); @@ -1863,14 +1863,14 @@ bool GIT::retrieve(FileName const & file) { LYXERR(Debug::LYXVC, "LyXVC::GIT: retrieve.\n\t" << file); // The caller ensures that file does not exist, so no need to check that. - return doVCCommandCall("git checkout -q " + quoteName(file.onlyFileName()), + return doVCCommandCall("git checkout -q -- " + quoteName(file.onlyFileName()), file.onlyPath()) == 0; } void GIT::registrer(string const & /*msg*/) { - doVCCommand("git add " + quoteName(onlyFileName(owner_->absFileName())), + doVCCommand("git add -- " + quoteName(onlyFileName(owner_->absFileName())), FileName(owner_->filePath())); } @@ -1888,10 +1888,10 @@ string GIT::rename(support::FileName const & newFile, string const & msg) // confusing to the user to have two uncommitted files. FileName path(owner_->filePath()); string relFile(to_utf8(newFile.relPath(path.absFileName()))); - string cmd("git mv " + quoteName(onlyFileName(owner_->absFileName())) + + string cmd("git mv -- " + quoteName(onlyFileName(owner_->absFileName())) + ' ' + quoteName(relFile)); if (doVCCommand(cmd, path)) { - cmd = "git checkout -q " + + cmd = "git checkout -q -- " + quoteName(onlyFileName(owner_->absFileName())) + ' ' + quoteName(relFile); doVCCommand(cmd, path); @@ -1904,7 +1904,7 @@ string GIT::rename(support::FileName const & newFile, string const & msg) f.push_back(newFile); string log; if (checkIn(f, msg, log) != LyXVC::VCSuccess) { - cmd = "git checkout -q " + + cmd = "git checkout -q -- " + quoteName(onlyFileName(owner_->absFileName())) + ' ' + quoteName(relFile); doVCCommand(cmd, path); @@ -1990,7 +1990,7 @@ bool GIT::isCheckInWithConfirmation() return true; } - doVCCommandCall("git diff " + quoteName(owner_->absFileName()) + doVCCommandCall("git diff -- " + quoteName(owner_->absFileName()) + " > " + quoteName(tmpf.toFilesystemEncoding()), FileName(owner_->filePath())); @@ -2069,7 +2069,7 @@ bool GIT::revert() // gets the updated version from the repository. string const fil = quoteName(onlyFileName(owner_->absFileName())); - if (doVCCommand("git checkout -q " + fil, + if (doVCCommand("git checkout -q -- " + fil, FileName(owner_->filePath()))) return false; owner_->markClean(); @@ -2150,7 +2150,7 @@ bool GIT::getFileRevisionInfo() return false; } - doVCCommand("git log -n 1 --pretty=format:%H%n%h%n%an%n%ai " + quoteName(onlyFileName(owner_->absFileName())) + doVCCommand("git log -n 1 --pretty=format:%H%n%h%n%an%n%ai -- " + quoteName(onlyFileName(owner_->absFileName())) + " > " + quoteName(tmpf.toFilesystemEncoding()), FileName(owner_->filePath())); @@ -2203,7 +2203,7 @@ bool GIT::getTreeRevisionInfo() void GIT::getLog(FileName const & tmpf) { - doVCCommand("git log " + quoteName(onlyFileName(owner_->absFileName())) + doVCCommand("git log -- " + quoteName(onlyFileName(owner_->absFileName())) + " > " + quoteName(tmpf.toFilesystemEncoding()), FileName(owner_->filePath())); } -- 2.39.5
-- lyx-devel mailing list [email protected] https://lists.lyx.org/mailman/listinfo/lyx-devel
