mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-12-29 17:00:58 +01:00
(experimental) Work around a Windows escaping issue and security conecern in "hg cat --output ..."
Summary: See PHI904. Ref T13210. Ref T13209. Currently, we have an `hg cat` construction which attempts to pass a literal `%p` to Mercurial. This fails because you can't pass `%` through `%s` outside of `wilds`. It also uses `%C` to pass a list of file paths. This is broadly unsafe and can cause command execution if you modify a file named, e.g., `; rm -rf xyz` or similar. I think it would be difficult to turn this into an attack but it's fairly bad. This dates from D5144 in 2013. Test Plan: With this patch, created D19757 which has valid binary data (see F5962134). Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13210, T13209 Differential Revision: https://secure.phabricator.com/D19758
This commit is contained in:
parent
9d59e9e590
commit
637c6584c6
1 changed files with 8 additions and 4 deletions
|
@ -460,12 +460,16 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
|
||||||
Filesystem::createDirectory(dirname($tmppath), 0755, true);
|
Filesystem::createDirectory(dirname($tmppath), 0755, true);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// NOTE: The "%s%%p" construction passes a literal "%p" to Mercurial,
|
||||||
|
// which is a formatting directive for a repo-relative filepath. The
|
||||||
|
// particulars of the construction avoid Windows escaping issues. See
|
||||||
|
// PHI904.
|
||||||
|
|
||||||
list($err, $stdout) = $this->execManualLocal(
|
list($err, $stdout) = $this->execManualLocal(
|
||||||
'cat --rev %s --output %s -- %C',
|
'cat --rev %s --output %s%%p -- %Ls',
|
||||||
$revision,
|
$revision,
|
||||||
// %p is the formatter for the repo-relative filepath
|
$tmpdir.DIRECTORY_SEPARATOR,
|
||||||
$tmpdir.'/%p',
|
$paths);
|
||||||
implode(' ', $paths));
|
|
||||||
|
|
||||||
$filedata = array();
|
$filedata = array();
|
||||||
foreach ($paths as $path) {
|
foreach ($paths as $path) {
|
||||||
|
|
Loading…
Reference in a new issue