4 ms·

Well, it's known as a "command injection". It's a common vulnerability that can happen in all languages. https://owasp.org/www-community/attacks/Command_Inject

Well, it's known as a "command injection". It's a common vulnerability that can happen in all languages. https://owasp.org/www-community/attacks/Command_Injection https://owasp.org/www-community/attacks/Command_Injection

amelius 5y ago
> It's a common vulnerability that can happen in all languages. From the article: > I would have written this in Rust, but, alas, it’s not vulnerable to this particular attack :) But of course, strictly speaking, this isn't the case, as you can always call sh -c and rust doest prevent you from doing that (though makes it harder).
- detaro 5y ago
  There is nothing fundamental about Rust that prevents command injection vulnerabilities. (What Rust has going for it in this specific case is that the standard library AFAIK doesn't have an easy way of doing it accidentally laying around - if you want to call the shell, you need to do it explicitly)
  - hnlmorg 5y ago
    But that's exactly the point. I'm often the first to tell people that Rust doesn't automatically guarantee safer code however in this instance Rust (and others who don't call `sh` by default) are safer because the developers have to explicitly write code to fork a shell. And if they're explicitly doing that then you'd fucking hope they'd understand the risks of doing so (and at least it would stand out more in a code peer review too). Whereas standard libraries that silently fork sh are less safer because they introduce the shell injection vulnerability by default thus requiring developers to consciously be aware of the need to sanitise their inputs. As I've mentioned elsewhere, I personally think it was a boneheaded decision to ever make exec() functions call a shell. Maybe I'll forgive Perl because that has it's origins as being an extension to shell scripting. But by the time node.js was released people really should have known better.
    - bregma 5y ago
      So you're saying Rust is safe because it does exactly what C does?
      - detaro 5y ago
        That's clearly not what they are saying, given that the C standard library contains (only contains, the safer ones are platform-dependent) the vulnerable form.
      - tialaramex 5y ago
        Where C does things that are a good idea (and there are some) they don't stop being good ideas just because C did them.
hnlmorg 5y ago
> It's a common vulnerability that can happen in all languages. While that's true, some language's standard libraries don't expose this vulnerability so the developer has to explicitly write bad code. And that's what really matters. Take node.js for example: it's not going to be obvious that `exec(command)` is actually equivalent to `sh -c "$command"` and thus a lot of people will get caught short if they have unescaped shell script tokens (eg $, <, >, ;, etc) in their command string. And that's very easy to happen. Personally I think it was a boneheaded decision when most of the languages adopted that approach of forking to sh. If developers wanted to fork to a shell then they could still explicitly write the code to do so (like the `spawn("/bin/sh", "-c", cmd)` example in the article). But by having exec fork sh by default you're creating a risk for all sorts of unexpected behaviour from developers who don't know their language's standard library inside out (which, lets be honest, is going to be most developers given the point of standard libraries is to abstract away that complexity so developers don't need to think about it). This isn't just a web scale problem either. In fact the example given in that article is a perfect one because its a CI/CD pipeline running trusted input that still failed unexpectedly.
- Galanwe 5y ago
  Cannot agree more. To me it seems the original program in the article should be correct, given the `exec` from node would indeed perform a simple exec, instead of actually being an `exec_shell` function.

amelius 5y ago

> It's a common vulnerability that can happen in all languages. From the article: > I would have written this in Rust, but, alas, it’s not vulnerable to this particular attack :) But of course, strictly speaking, this isn't the case, as you can always call sh -c and rust doest prevent you from doing that (though makes it harder).

detaro 5y ago
There is nothing fundamental about Rust that prevents command injection vulnerabilities. (What Rust has going for it in this specific case is that the standard library AFAIK doesn't have an easy way of doing it accidentally laying around - if you want to call the shell, you need to do it explicitly)
- hnlmorg 5y ago
  But that's exactly the point. I'm often the first to tell people that Rust doesn't automatically guarantee safer code however in this instance Rust (and others who don't call `sh` by default) are safer because the developers have to explicitly write code to fork a shell. And if they're explicitly doing that then you'd fucking hope they'd understand the risks of doing so (and at least it would stand out more in a code peer review too). Whereas standard libraries that silently fork sh are less safer because they introduce the shell injection vulnerability by default thus requiring developers to consciously be aware of the need to sanitise their inputs. As I've mentioned elsewhere, I personally think it was a boneheaded decision to ever make exec() functions call a shell. Maybe I'll forgive Perl because that has it's origins as being an extension to shell scripting. But by the time node.js was released people really should have known better.
  - bregma 5y ago
    So you're saying Rust is safe because it does exactly what C does?
    - detaro 5y ago
      That's clearly not what they are saying, given that the C standard library contains (only contains, the safer ones are platform-dependent) the vulnerable form.
    - tialaramex 5y ago
      Where C does things that are a good idea (and there are some) they don't stop being good ideas just because C did them.

hnlmorg 5y ago

> It's a common vulnerability that can happen in all languages. While that's true, some language's standard libraries don't expose this vulnerability so the developer has to explicitly write bad code. And that's what really matters. Take node.js for example: it's not going to be obvious that `exec(command)` is actually equivalent to `sh -c "$command"` and thus a lot of people will get caught short if they have unescaped shell script tokens (eg $, <, >, ;, etc) in their command string. And that's very easy to happen. Personally I think it was a boneheaded decision when most of the languages adopted that approach of forking to sh. If developers wanted to fork to a shell then they could still explicitly write the code to do so (like the `spawn("/bin/sh", "-c", cmd)` example in the article). But by having exec fork sh by default you're creating a risk for all sorts of unexpected behaviour from developers who don't know their language's standard library inside out (which, lets be honest, is going to be most developers given the point of standard libraries is to abstract away that complexity so developers don't need to think about it). This isn't just a web scale problem either. In fact the example given in that article is a perfect one because its a CI/CD pipeline running trusted input that still failed unexpectedly.

Galanwe 5y ago
Cannot agree more. To me it seems the original program in the article should be correct, given the `exec` from node would indeed perform a simple exec, instead of actually being an `exec_shell` function.