5 ms·

I think load and dangerous_load is a wonderful paradigm for API design. It bakes security into the core use of your framework for non-security devs. It also m

by patio11 14y ago

I think load and dangerous_load is a wonderful paradigm for API design. It bakes security into the core use of your framework for non-security devs. It also makes the framework more productive for security professionals, since for an assessment you can grep for dangerous_load and focus your efforts on making sure those (rare) calls are hermetically sealed from user input rather than having to audit every possible use of the 100x more common load command. (That is where the Rails community is right now, and it sucks.) This is similar to requiring a whitelist for mass assignment (rather than making a blacklist optional): it calls out in the code "These are our weak points! Check them carefully!"

thibaut_barrere 14y ago
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.
untog 14y ago
I think load and dangerous_load is a wonderful paradigm for API design. It is, but I think it ignores the reality of API design. Who would ever start with dangerous_load? If you knew it was dangerous, you'd probably fix it. No, you start with load(), then find out it's dangerous afterwards. The fix will be a breaking change, so you make safe_load(). "You should replace load()!", I hear you cry. But that removes backwards compatibility. And people don't update legacy software. So then old software becomes vulnerable to who knows how many other security problems because of it's obsolescence.
- vog 14y ago
  > ... ignores the reality of API design. Who would ever start with dangerous_load? When I wrote the Texcaller library, which simplifies compiling (La)TeX code, I disabled dangerous features such as "write18" from the very beginning. I you don't take basic elements such as "secure/unsecure" variants into account, you're no really doing API design. You then just have a historically grown API, which is not necessarily bad in itself, but doesn't qualify to be called API design. > But that removes backwards compatibility. And people don't update legacy software. So then old software becomes vulnerable to who knows how many other security problems because of it's obsolescence. In that case, at least only old hard-to-update legacy software is affected, and not all the other (good, mostly well-written) software that is written today and in the future. Of course, don't forget to increase the major version number, as for every API change that breaks backward compatibility.
  - untog 14y ago
    doesn't qualify to be called API design. True. I should have said the reality of the difficulty of API design. In that case, at least only old hard-to-update legacy software is affected, and not all the other (good, mostly well-written) software that is written today and in the future. I like the sentiment, but I can't agree with it. In an ideal world all critical systems would always be kept up the date, but that doesn't happen. Has Python 3 usage overtaken 2.x yet?
- ufo 14y ago
  > Who would ever start with dangerous_load? Haskellers. This is why you get lovely long and alarming names like `unsafePerformIO`
vq 14y ago
This is often the case in Haskell APIs, easily abusable or functions with hard to predict side-effects are usually prefixed with "unsafe", for example unsafeCoerce or unsafePerformIO.
- tikhonj 14y ago
  Another cool thing, from a security perspective, is that there is actually a tool (Safe Haskell[1]) that makes it easy to disallow unsafe behavior and operate a whitelist of trusted modules. So not only is unsafePerformIO well-named, it's also trivial to restrict its usage. [1]: http://www.haskell.org/ghc/docs/7.4.1/html/users_guide/safe-haskell.html http://www.haskell.org/ghc/docs/7.4.1/html/users_guide/safe-...
SilasX 14y ago
So I guess the STD/STI prevention community was actually ahead of the computer security community in having the term "safer sex" replace "safe sex"?
nbm 14y ago
This "encode problems into names" approach is something I first ran into at Facebook. When a particular function or class is found to be a problem (or when it is known to be a problem in advance), it is (re)named something like "foo_POTENTIAL_XSS_HOLE" or "FooMapNonStlCompliant" throughout the code base (using pfff or similar tool), and then the potentially long process of removing the problem can go forward with some confidence nobody will add to the problem while you're working on it. pfff: https://github.com/facebook/pfff https://github.com/facebook/pfff
speleding 14y ago
I realise you probably meant this for the more general case, but if you are building an API it should not accept YAML. If you are not building an API and are using YAML to de-serialze stuff you serialised yourself (i.e. use YAML as intended) then there is no issue anyway. So having "dangerous_load" is not going to help much with YAML: if it's exposed to untrusted input then grep for "YAML" not for "dangerous_load".

by patio11 14y ago

thibaut_barrere 14y ago

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.

untog 14y ago

I think load and dangerous_load is a wonderful paradigm for API design. It is, but I think it ignores the reality of API design. Who would ever start with dangerous_load? If you knew it was dangerous, you'd probably fix it. No, you start with load(), then find out it's dangerous afterwards. The fix will be a breaking change, so you make safe_load(). "You should replace load()!", I hear you cry. But that removes backwards compatibility. And people don't update legacy software. So then old software becomes vulnerable to who knows how many other security problems because of it's obsolescence.

vog 14y ago
> ... ignores the reality of API design. Who would ever start with dangerous_load? When I wrote the Texcaller library, which simplifies compiling (La)TeX code, I disabled dangerous features such as "write18" from the very beginning. I you don't take basic elements such as "secure/unsecure" variants into account, you're no really doing API design. You then just have a historically grown API, which is not necessarily bad in itself, but doesn't qualify to be called API design. > But that removes backwards compatibility. And people don't update legacy software. So then old software becomes vulnerable to who knows how many other security problems because of it's obsolescence. In that case, at least only old hard-to-update legacy software is affected, and not all the other (good, mostly well-written) software that is written today and in the future. Of course, don't forget to increase the major version number, as for every API change that breaks backward compatibility.
- untog 14y ago
  doesn't qualify to be called API design. True. I should have said the reality of the difficulty of API design. In that case, at least only old hard-to-update legacy software is affected, and not all the other (good, mostly well-written) software that is written today and in the future. I like the sentiment, but I can't agree with it. In an ideal world all critical systems would always be kept up the date, but that doesn't happen. Has Python 3 usage overtaken 2.x yet?
ufo 14y ago
> Who would ever start with dangerous_load? Haskellers. This is why you get lovely long and alarming names like `unsafePerformIO`

vq 14y ago

This is often the case in Haskell APIs, easily abusable or functions with hard to predict side-effects are usually prefixed with "unsafe", for example unsafeCoerce or unsafePerformIO.

tikhonj 14y ago
Another cool thing, from a security perspective, is that there is actually a tool (Safe Haskell[1]) that makes it easy to disallow unsafe behavior and operate a whitelist of trusted modules. So not only is unsafePerformIO well-named, it's also trivial to restrict its usage. [1]: http://www.haskell.org/ghc/docs/7.4.1/html/users_guide/safe-haskell.html http://www.haskell.org/ghc/docs/7.4.1/html/users_guide/safe-...

SilasX 14y ago

So I guess the STD/STI prevention community was actually ahead of the computer security community in having the term "safer sex" replace "safe sex"?

nbm 14y ago

This "encode problems into names" approach is something I first ran into at Facebook. When a particular function or class is found to be a problem (or when it is known to be a problem in advance), it is (re)named something like "foo_POTENTIAL_XSS_HOLE" or "FooMapNonStlCompliant" throughout the code base (using pfff or similar tool), and then the potentially long process of removing the problem can go forward with some confidence nobody will add to the problem while you're working on it. pfff: https://github.com/facebook/pfff https://github.com/facebook/pfff

speleding 14y ago

I realise you probably meant this for the more general case, but if you are building an API it should not accept YAML. If you are not building an API and are using YAML to de-serialze stuff you serialised yourself (i.e. use YAML as intended) then there is no issue anyway. So having "dangerous_load" is not going to help much with YAML: if it's exposed to untrusted input then grep for "YAML" not for "dangerous_load".