5 ms·

I suggest Rails people have a look at safe_yaml which (now) replaces YAML.load by a safe (safer?) version: https://github.com/dtao/safe_yaml https://github.com

I suggest Rails people have a look at safe_yaml which (now) replaces YAML.load by a safe (safer?) version: https://github.com/dtao/safe_yaml https://github.com/dtao/safe_yaml (nb: I've enable it on apps without issues)

patio11 14y ago
I like that project but have been trying to avoid saying "Use this!" because I haven't personally audited it and am scared that Rails devs might adopt it and think it means it definitively resolves all of their issues for February, which it might not.
- thibaut_barrere 14y ago
  So far, people I've seen using it are "security-aware" developers who know there is always a remaining risk and who are just trying to reduce the surface attack a bit. For the others, well, we'll have to educate them, or push a safe yaml default quickly into Ruby [1]. This is no holy grail for sure, and there are plenty of other topics to be addressed (eg: secret tokens stored in SCM, pushed to third-party CI services and shared with freelancers and remote employees using non-encrypted disk, shared as well between production and non-regularly updated staging servers etc!) [1] https://github.com/tenderlove/psych/issues/119 https://github.com/tenderlove/psych/issues/119
PommeDeTerre 14y ago
This reminds me a lot of how the PHP crowd handles these kinds of problems. So there's some piece of functionality, but security or some other obviously-important factor wasn't considered at all when it was initially "designed" and implemented. It's soon found to exhibit numerous problems, often including major security flaws. Then a "safe" version of said functionality is offered. Yet for some reason (incompetence?) it has its own set of problems. And so the developers keep trying again and again, never seeming to get anywhere close to even a suitable solution. An example is the mysql_escape_string() function, which was replaced by mysql_real_escape_string(), which has in turn been replaced by mysqli_escape_string(), mysqli::real_escape_string() and PDO::quote(). The end result is confusion, especially for novice users, or those coming back to PHP after some time. They don't know which of the several functions to use, or they're using older reference material that suggest the use of the faultiest of the functions. It's better just to implement things properly the first time around, especially when security or data integrity, for example, are involved. Trying to hack on "safe" versions of functions, or even an entire "safe mode" is the wrong approach.
- knowtheory 14y ago
  I have never, ever, not even once, in my 8 years working with rails had to call YAML.load myself in an app. Changing how Rails or other libs process YAML is a behavior that can and should be performed transparently to users. And besides, most people processing user input aren't accepting YAML. It is extremely weird that Rails was accepting YAML in XML requests. It is slightly more understandable that Rails was using a YAML parser to parse JSON (although not a good idea as we've seen). But by and large, if you have an API that accepts user data, standard practice is to parse JSON, not YAML. Your analogy is not correct.
  - SilasX 14y ago
    I never thought I'd see "Rails" and "transparent" in the same sentence...
- lukeschlather 14y ago
  Composing Mysql manually is a Bad Idea, and the safe version is not safe because the algorithm is fundamentally flawed. People need to be doing parametrized queries. The reason novices can't adapt to PHP is that the library design is bad to begin with. I don't think either Python or Ruby has this problem. I don't think anyone will be confused because suddenly YAML.load doesn't execute arbitrary code.
  - jrochkind1 14y ago
    > I don't think anyone will be confused because suddenly YAML.load doesn't execute arbitrary code. An example of where people would be confused because YAML.load doesn't _instantiate arbitrary objects_ (which is what it really does, which results in ability to 'execute arbitrary code' as a poorly thought through side effect) -- is people using ActiveRecord::Base.serialize . Which would become broken if you were serializing any objects that weren't string, hash, integer, array. While we've realized that allowing de-serialization of arbitrary objects ends up being incredibly likely to result in 'allowing execution of arbitrary code' -- referring to the problem simply as the latter confuses about the nature of the problem and the efficacy of various fixes.

patio11 14y ago

I like that project but have been trying to avoid saying "Use this!" because I haven't personally audited it and am scared that Rails devs might adopt it and think it means it definitively resolves all of their issues for February, which it might not.

thibaut_barrere 14y ago
So far, people I've seen using it are "security-aware" developers who know there is always a remaining risk and who are just trying to reduce the surface attack a bit. For the others, well, we'll have to educate them, or push a safe yaml default quickly into Ruby [1]. This is no holy grail for sure, and there are plenty of other topics to be addressed (eg: secret tokens stored in SCM, pushed to third-party CI services and shared with freelancers and remote employees using non-encrypted disk, shared as well between production and non-regularly updated staging servers etc!) [1] https://github.com/tenderlove/psych/issues/119 https://github.com/tenderlove/psych/issues/119

PommeDeTerre 14y ago

This reminds me a lot of how the PHP crowd handles these kinds of problems. So there's some piece of functionality, but security or some other obviously-important factor wasn't considered at all when it was initially "designed" and implemented. It's soon found to exhibit numerous problems, often including major security flaws. Then a "safe" version of said functionality is offered. Yet for some reason (incompetence?) it has its own set of problems. And so the developers keep trying again and again, never seeming to get anywhere close to even a suitable solution. An example is the mysql_escape_string() function, which was replaced by mysql_real_escape_string(), which has in turn been replaced by mysqli_escape_string(), mysqli::real_escape_string() and PDO::quote(). The end result is confusion, especially for novice users, or those coming back to PHP after some time. They don't know which of the several functions to use, or they're using older reference material that suggest the use of the faultiest of the functions. It's better just to implement things properly the first time around, especially when security or data integrity, for example, are involved. Trying to hack on "safe" versions of functions, or even an entire "safe mode" is the wrong approach.

knowtheory 14y ago
I have never, ever, not even once, in my 8 years working with rails had to call YAML.load myself in an app. Changing how Rails or other libs process YAML is a behavior that can and should be performed transparently to users. And besides, most people processing user input aren't accepting YAML. It is extremely weird that Rails was accepting YAML in XML requests. It is slightly more understandable that Rails was using a YAML parser to parse JSON (although not a good idea as we've seen). But by and large, if you have an API that accepts user data, standard practice is to parse JSON, not YAML. Your analogy is not correct.
- SilasX 14y ago
  I never thought I'd see "Rails" and "transparent" in the same sentence...
lukeschlather 14y ago
Composing Mysql manually is a Bad Idea, and the safe version is not safe because the algorithm is fundamentally flawed. People need to be doing parametrized queries. The reason novices can't adapt to PHP is that the library design is bad to begin with. I don't think either Python or Ruby has this problem. I don't think anyone will be confused because suddenly YAML.load doesn't execute arbitrary code.
- jrochkind1 14y ago
  > I don't think anyone will be confused because suddenly YAML.load doesn't execute arbitrary code. An example of where people would be confused because YAML.load doesn't _instantiate arbitrary objects_ (which is what it really does, which results in ability to 'execute arbitrary code' as a poorly thought through side effect) -- is people using ActiveRecord::Base.serialize . Which would become broken if you were serializing any objects that weren't string, hash, integer, array. While we've realized that allowing de-serialization of arbitrary objects ends up being incredibly likely to result in 'allowing execution of arbitrary code' -- referring to the problem simply as the latter confuses about the nature of the problem and the efficacy of various fixes.