commit f7e05eb13aa9513ba3a9ddb49e65dffe12419dd4
Author: Pavel Sanda <[email protected]>
Date: Sun Oct 11 21:55:36 2026 +0200
VCS: block argument smuggling
Insert " -- " between every git, svn and cvs 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
* RCS - has no functional --, handled separately
Tier 01 DiD / vcs-args: -- separator for VCS filename arguments
https://www.mail-archive.com/[email protected]/msg227023.html
Assisted-by: Claude Opus 5
---
src/VCBackend.cpp | 86 +++++++++++++++++++++++++++----------------------------
1 file changed, 43 insertions(+), 43 deletions(-)
diff --git a/src/VCBackend.cpp b/src/VCBackend.cpp
index 751529c9be..897c35489f 100644
--- a/src/VCBackend.cpp
+++ b/src/VCBackend.cpp
@@ -612,7 +612,7 @@ bool CVS::retrieve(FileName const & file)
{
LYXERR(Debug::LYXVC, "LyXVC::CVS: retrieve.\n\t" << file);
// The caller ensures that file does not exist, so no need to check
that.
- return doVCCommandCall("cvs -q update " +
quoteName(file.toFilesystemEncoding()),
+ return doVCCommandCall("cvs -q update -- " +
quoteName(file.toFilesystemEncoding()),
file.onlyPath()) == 0;
}
@@ -679,7 +679,7 @@ CVS::CvsStatus CVS::getStatus()
return StatusError;
}
- if (doVCCommandCallWithOutput("cvs status " + getTarget(File),
+ if (doVCCommandCallWithOutput("cvs status -- " + getTarget(File),
FileName(owner_->filePath()), tmpf)) {
return StatusError;
}
@@ -720,7 +720,7 @@ void CVS::getRevisionInfo()
}
int rc = doVCCommandCallWithOutput("cvs log -r" + version_
- + ' ' + getTarget(File),
+ + " -- " + getTarget(File),
FileName(owner_->filePath()), tmpf);
if (rc) {
LYXERR(Debug::LYXVC, "cvs log failed with exit code " << rc);
@@ -756,7 +756,7 @@ void CVS::getRevisionInfo()
void CVS::registrer(string const & msg)
{
- doVCCommand("cvs -q add -m \"" + msg + "\" "
+ doVCCommand("cvs -q add -m \"" + msg + "\" -- "
+ getTarget(File),
FileName(owner_->filePath()));
}
@@ -775,7 +775,7 @@ string CVS::rename(support::FileName const & newFile,
string const & msg)
string ret = copy(newFile, msg);
if (ret.empty())
return ret;
- string cmd = "cvs -q remove -m \"" + msg + "\" " +
+ string cmd = "cvs -q remove -m \"" + msg + "\" -- " +
quoteName(oldFile.onlyFileName());
FileName path(oldFile.onlyPath());
return doVCCommand(cmd, path) ? string() : ret;
@@ -796,14 +796,14 @@ string CVS::copy(support::FileName const & newFile,
string const & msg)
return string();
FileName path(oldFile.onlyPath());
string relFile(to_utf8(newFile.relPath(path.absFileName())));
- string cmd("cvs -q add -m \"" + msg + "\" " + quoteName(relFile));
+ string cmd("cvs -q add -m \"" + msg + "\" -- " + quoteName(relFile));
return doVCCommand(cmd, path) ? string() : "CVS: Proceeded";
}
void CVS::getDiff(OperationMode opmode, FileName const & tmpf)
{
- doVCCommandWithOutput("cvs diff " + getTarget(opmode),
+ doVCCommandWithOutput("cvs diff -- " + getTarget(opmode),
FileName(owner_->filePath()), tmpf, false);
}
@@ -811,7 +811,7 @@ void CVS::getDiff(OperationMode opmode, FileName const &
tmpf)
int CVS::edit()
{
vcstatus_ = LOCKED;
- return doVCCommand("cvs -q edit " + getTarget(File),
+ return doVCCommand("cvs -q edit -- " + getTarget(File),
FileName(owner_->filePath()));
}
@@ -819,14 +819,14 @@ int CVS::edit()
int CVS::unedit()
{
vcstatus_ = UNLOCKED;
- return doVCCommand("cvs -q unedit " + getTarget(File),
+ return doVCCommand("cvs -q unedit -- " + getTarget(File),
FileName(owner_->filePath()));
}
int CVS::update(OperationMode opmode, FileName const & tmpf)
{
- return doVCCommandWithOutput("cvs -q update "
+ return doVCCommandWithOutput("cvs -q update -- "
+ getTarget(opmode),
FileName(owner_->filePath()), tmpf, false);
}
@@ -864,7 +864,7 @@ LyXVC::CommandResult CVS::checkIn(string const & msg,
string & log)
return LyXVC::VCSuccess;
case LocallyModified:
case LocallyAdded: {
- int rc = doVCCommand("cvs -q commit -m \"" + msg + "\" "
+ int rc = doVCCommand("cvs -q commit -m \"" + msg + "\" -- "
+ getTarget(File),
FileName(owner_->filePath()));
if (rc)
@@ -1082,7 +1082,7 @@ bool CVS::undoLastEnabled()
void CVS::getLog(FileName const & tmpf)
{
- doVCCommandWithOutput("cvs log " + getTarget(File),
+ doVCCommandWithOutput("cvs log -- " + getTarget(File),
FileName(owner_->filePath()),
tmpf);
}
@@ -1129,7 +1129,7 @@ bool CVS::prepareFileRevision(string const & revis,
string & f)
return false;
}
- doVCCommandWithOutput("cvs update -p -r" + rev + ' '
+ doVCCommandWithOutput("cvs update -p -r" + rev + " -- "
+ getTarget(File),
FileName(owner_->filePath()), tmpf);
tmpf.refresh();
@@ -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()));
}
--
lyx-cvs mailing list
[email protected]
https://lists.lyx.org/mailman/listinfo/lyx-cvs
