Page MenuHomePhabricator

Filter and reject "--config" and "--debugger" flags to Mercurial in any position

Authored by epriestley on Nov 10 2017, 4:22 PM.



Ref T13012. These flags can be exploited by attackers to execute code remotely. See T13012 for discussion and context.

Additionally, harden some Mercurial commands where possible (by using additional quoting or embedding arguments in other constructs) so they resist these flags and behave properly when passed arguments with these values.

Test Plan
  • Added unit tests.
  • Verified "--config" and "--debugger" commands are rejected.
  • Verified more commands now work properly even with branches and files named --debugger, although not all of them do.

Diff Detail

rP Phabricator
Lint Not Applicable
Tests Not Applicable

Event Timeline

amckinley added inline comments.

I haven't dug into the guts of PhutilShellLexer, but is it robust enough to catch people doing something like echo "--deb" | echo "ugger" or whatever?


Worth logging this so we can detect malicious users?

This revision is now accepted and ready to land.Nov 10 2017, 4:33 PM

It can't, but if users can control echo ... | echo ... they can presumably just execute wget | sh instead.

It only handles the basic quoting rules, but we've applied all the quoting ourselves, and it handles (or, at least, should handle) those properly (see PhutilShellLexerTestCase.php in libphutil/).

Or, put another way: if it doesn't handle an input, we almost certainly already have a separate RCE vulnerability with a different source that isn't related to Mercurial.


I don't know what we'd do with this knowledge offhand, but if you want to log it in the cluster you can add a subclass of PhabricatorRequestExceptionHandler to services/.

It could do this in canHandleRequestThrowable() to just dump them to the log without interrupting processing.

return false;
This revision was automatically updated to reflect the committed changes.