5 ms·

How is this any different than a SQL-injection attack? Seems to me like git shouldn't be using the shell in order to invoke ssh at all.

jjnoakes 9y ago
I haven't checked git, but even if git uses exec*() directly and skips the shell (which I suspect it does), the problem still exists. The issue isn't one of shell quoting, but one of ssh treating the argument list ["ssh", "-oProxyCommand=something", "remote-cmd"] not as ["ssh", hostname, command] but as ["ssh", option, hostname].
- deckar01 9y ago
  There are C libraries for SSH, like libssh, that would avoid these type of vulnerabilities. https://www.libssh.org/ https://www.libssh.org/
  - peff 9y ago
    One downside of libraries like libssh is that they don't behave the same way as your regular ssh command. So thing you've configured like host aliases, proxy commands, etc, don't just work out of the box (and in some cases may not even be supported at all).
    - deckar01 9y ago
      True. It sounds like there is room for an ORM-like sanitizer for SSH that is responsible for translating a C API to sanitized commands. I have no clue what to search to find if that already exists.
      - virtualized 9y ago
        It almost seems like ad-hoc text formats like command line argument syntaxes are difficult to use correctly.
  - snakeanus 9y ago
    libgit2 depends on libssh2 actually.
- numbsafari 9y ago
  Got it. Seems to me the the use of "=" notation for CLI args would be rife with situations like this. Yikes... [also, still going through the code, but it looks like they are setting up for a call to exec*()]
  - wahern 9y ago
    Yes, which is why "--" is the standard way to avoid this stuff, and why people should generally avoid writing their own options processor and instead use getopt() or getopt_long(), which provide consistent, standard semantics.
- peff 9y ago
  Exactly. It's an option injection attack. There's no shell involved.
rschoon 9y ago
In this case, it's not the shell that's the problem, it's ssh's command line argument parsing that you need to be careful with.
- nobodyorother 9y ago
  Not just SSH's library parsing, but every SSH implementation's argument parsing.

jjnoakes 9y ago

I haven't checked git, but even if git uses exec*() directly and skips the shell (which I suspect it does), the problem still exists. The issue isn't one of shell quoting, but one of ssh treating the argument list ["ssh", "-oProxyCommand=something", "remote-cmd"] not as ["ssh", hostname, command] but as ["ssh", option, hostname].

deckar01 9y ago
There are C libraries for SSH, like libssh, that would avoid these type of vulnerabilities. https://www.libssh.org/ https://www.libssh.org/
- peff 9y ago
  One downside of libraries like libssh is that they don't behave the same way as your regular ssh command. So thing you've configured like host aliases, proxy commands, etc, don't just work out of the box (and in some cases may not even be supported at all).
  - deckar01 9y ago
    True. It sounds like there is room for an ORM-like sanitizer for SSH that is responsible for translating a C API to sanitized commands. I have no clue what to search to find if that already exists.
    - virtualized 9y ago
      It almost seems like ad-hoc text formats like command line argument syntaxes are difficult to use correctly.
- snakeanus 9y ago
  libgit2 depends on libssh2 actually.
numbsafari 9y ago
Got it. Seems to me the the use of "=" notation for CLI args would be rife with situations like this. Yikes... [also, still going through the code, but it looks like they are setting up for a call to exec*()]
- wahern 9y ago
  Yes, which is why "--" is the standard way to avoid this stuff, and why people should generally avoid writing their own options processor and instead use getopt() or getopt_long(), which provide consistent, standard semantics.
peff 9y ago
Exactly. It's an option injection attack. There's no shell involved.

rschoon 9y ago

In this case, it's not the shell that's the problem, it's ssh's command line argument parsing that you need to be careful with.

nobodyorother 9y ago
Not just SSH's library parsing, but every SSH implementation's argument parsing.