3 ms·

I understand that it is convenient for running small snippets like that, but I don't really think it's worth the risk. And putting it into a config file is diff

by panzi 1y ago

I understand that it is convenient for running small snippets like that, but I don't really think it's worth the risk. And putting it into a config file is different, IMO. You don't get tempted to do some bad string interpolation there, because you can't, unless the config file format has support for that, but then I criticize that. If you need to pass things to such a snipped do it via environment variables or standard IO, not string interpolation. If you say you don't make such mistakes: Yeah, but people do. People that write the code that runs on your system.

theamk 1y ago
But if you want a command-line option for hook, what are the alternatives? Force user to always create a wrapper script? that's just extra annoyance and if user is bad at quoting, they'll have the same problems with a script Disable hooks at all? that's bad functionality regression Ask for multiple arguments? this makes command-line parsing much more awkward.. I have not seen any good solutions for that. (The only exception is writing a command wrapper that takes exactly 1 user command, like "timeout" or "xargs".. but those already using argument vector instead of parsing)
- frumplestlatz 1y ago
  You define a config file format that supports only the minimal syntax required to specify a multi-argument command (e.g. spaces separate arguments, arguments with spaces in them may be quoted or use backslashes to escape them). Then, you parse that out into a proper argument array and pass it to exec*/posix_spawn.
  - account42 1y ago
    So instead of a well known (i.e. POSIX) quoting semantics and existing tool support, you want to introduce your own ad-hoc format? No thanks.
    - frumplestlatz 1y ago
      A correct parser for the syntax I described can be written in less than a 100 lines of code — even in C. It’s a strict subset of the shell command language defined by POSIX, and it’s sufficiently expressive as to support specifying any argument array unambiguously. To correctly escape arbitrary shell syntax, not only do you need to handle the full POSIX syntax (which is quite complex) … https://pubs.opengroup.org/onlinepubs/9799919799/utilities/V3_chap02.html#tag_19_02 https://pubs.opengroup.org/onlinepubs/9799919799/utilities/V... … but you must also cover any bugs and undocumented/underspecified extensions implemented by the actual shell providing /bin/sh on every platform and platform version to which your code will be deployed. That’s not just difficult — it’s impossible, and everyone that has tried has failed, repeatedly. Leading to security bugs, repeatedly. https://gist.github.com/Zenexer/40d02da5e07f151adeaeeaa11af9ab36 https://gist.github.com/Zenexer/40d02da5e07f151adeaeeaa11af9... There’s a reason why we use parameterized queries instead of escaping to prevent SQL injection, and SQL syntax and parsing behavior is far more rigorously specified than the shell.
      - account42 1y ago
        You don't need to care about the whole quoting spec. E.g. what you need to escape without quotes becomes irrelevant as soon as you consistently add quotes and what you can do with $ expressions is irrelevant if you simply escape all $. This is far from rocket science. But it also doesn't matter how complex the standard is, it is a standard and one with much preexisting support. Your "standard" will end up being different for each application.
        frumplestlatz 1y ago
        Decades of CVEs say otherwise. Doing things the way you seem to prefer is just willful negligence at this point.
        oguz-ismail 1y ago
        This is fear mongering. Neither your comments nor the links you posted shows how rolling your custom command interpreter syntax is in any way better than using the ones available on your target platforms.

by panzi 1y ago

theamk 1y ago

But if you want a command-line option for hook, what are the alternatives? Force user to always create a wrapper script? that's just extra annoyance and if user is bad at quoting, they'll have the same problems with a script Disable hooks at all? that's bad functionality regression Ask for multiple arguments? this makes command-line parsing much more awkward.. I have not seen any good solutions for that. (The only exception is writing a command wrapper that takes exactly 1 user command, like "timeout" or "xargs".. but those already using argument vector instead of parsing)

frumplestlatz 1y ago
You define a config file format that supports only the minimal syntax required to specify a multi-argument command (e.g. spaces separate arguments, arguments with spaces in them may be quoted or use backslashes to escape them). Then, you parse that out into a proper argument array and pass it to exec*/posix_spawn.
- account42 1y ago
  So instead of a well known (i.e. POSIX) quoting semantics and existing tool support, you want to introduce your own ad-hoc format? No thanks.
  - frumplestlatz 1y ago
    A correct parser for the syntax I described can be written in less than a 100 lines of code — even in C. It’s a strict subset of the shell command language defined by POSIX, and it’s sufficiently expressive as to support specifying any argument array unambiguously. To correctly escape arbitrary shell syntax, not only do you need to handle the full POSIX syntax (which is quite complex) … https://pubs.opengroup.org/onlinepubs/9799919799/utilities/V3_chap02.html#tag_19_02 https://pubs.opengroup.org/onlinepubs/9799919799/utilities/V... … but you must also cover any bugs and undocumented/underspecified extensions implemented by the actual shell providing /bin/sh on every platform and platform version to which your code will be deployed. That’s not just difficult — it’s impossible, and everyone that has tried has failed, repeatedly. Leading to security bugs, repeatedly. https://gist.github.com/Zenexer/40d02da5e07f151adeaeeaa11af9ab36 https://gist.github.com/Zenexer/40d02da5e07f151adeaeeaa11af9... There’s a reason why we use parameterized queries instead of escaping to prevent SQL injection, and SQL syntax and parsing behavior is far more rigorously specified than the shell.
    - account42 1y ago
      You don't need to care about the whole quoting spec. E.g. what you need to escape without quotes becomes irrelevant as soon as you consistently add quotes and what you can do with $ expressions is irrelevant if you simply escape all $. This is far from rocket science. But it also doesn't matter how complex the standard is, it is a standard and one with much preexisting support. Your "standard" will end up being different for each application.
      - frumplestlatz 1y ago
        Decades of CVEs say otherwise. Doing things the way you seem to prefer is just willful negligence at this point.
        oguz-ismail 1y ago
        This is fear mongering. Neither your comments nor the links you posted shows how rolling your custom command interpreter syntax is in any way better than using the ones available on your target platforms.