Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Github Is Classy

jtimberman.housepub.org

Post reply on HN
Print view View original

51–60 of 70 posts

Re: Github Is Classy

#51

Post by aptwebapps »

javascriptlol wrote: post #48
aptwebapps wrote: post #47

Earlier quoted context omitted.

It's not sophistry, the distinction raganwald is making is relevant. However, even if it is not technically accurate, in the interest of getting the topic in front of as many Rails developers as possible, it's probably better to sweep that distinction under the rug and let them figure out for themselves whether it applies to them.

So your contention is that because something is "relevant" that prohibits the possibility of it being sophistic? The distinction is completely artificial. This bug in rails can be directly traced to recurring security problems. If that's not a vulnerability, then we speak a different dialect of English.

Ok, look, I actually think it is a vulnerability to most approximations which isn't what comes across in what I wrote.

That said, I don't think what raganwald was saying was sophistry at all. Sophistry implies an attempt to deceive. He was just being pedantic and a little narrow with his definition of vulnerability.

So when you say his argument sophistry, and then follow up with "... and I'm not interested in playing the "try to be right on the Internet" game with you." you're just lashing out. So that's probably why people (not me) where downvoting without replying.

Re: Github Is Classy

#52

Post by ryanbrunner »

raganwald wrote: post #19
jacques_chester wrote: post #16

Earlier quoted context omitted.

You confusing mechanism and policy . Sure, Rails includes a mechanism that closes this security hole. But it requires additional effort to apply a more secure policy . The security baseline is not elevated by the mechanism. Historical experience has shown that no matter how excellent the mechanism, the practical security baseline is determined by the default policy. Windows NT has had, since inception, a far more sop…

The way you describe it, it sounds like there is a hole in every app and developers must close the hole to be secure. My claim is that Rails provides a completely optional tool, mass assignment, that opens a hole, and another completely optional tool that closes the same hole. And they document this well. The one tool can be said to have a poor policy, I agree with that. But Rails does not have a hole in it. I did no…

The problem is, the fact that you don't use mass assignment is by far the exception rather than the rule. Every notable Rails tutorial uses mass assignment. Every person using InheritedResources is using mass assignment. GitHub is using mass assignment. Passing in params more or less raw from the controller, and leaving cleansing and validation up to the model is as strong an idiom as there is in Rails.

Regarding your ActiveRecord example, I would totally agree that there was a hole if the standard approach to running SQL was to use find_by_sql with string interpolation. Fortunately, that's appropriately viewed as an option of last resort, with appropriate amounts of "here be dragons" around the dangers of calling that method. Meanwhile, tutorials gleefully do mass-assignment, with perhaps a footnote about attr_accessible, which was of course not included for the sake of brevity.

It might be worth saying that Rails strictly as a framework does not have a security problem with mass assignment, only a very dangerous tool, but there is certainly a problem with the culture regarding it.

Re: Github Is Classy

#53

Post by jordanb »

raganwald wrote: post #17
timr wrote: post #10

Earlier quoted context omitted.

"this isn’t a security vulnerability in Rails. A new Rails application isn’t vulnerable by default. It’s a security vulnerability in apps written using a certain style, or perhaps a vulnerability by default in apps written using the generators and scaffolding baked into rails." Well, gee...so maybe some novice developers write an app using the generators and scaffolding baked into Rails , and as a result they get a s…

so maybe some novice developers write an app using the generators and scaffolding baked into Rails, and as a result they get a security vulnerability that a more experienced developer might have avoided by doing extra work, and you say that's not a security problem in Rails? Young man, do not take that flippant tone with me (raps ruler on desk). Those novice developers failed to RTFM. If you want a framework that pro…

I used to see this argument a lot from PHP people. You know, it's not PHP that's insecure, it's all those novices writing insecure PHP code.

Except that PHP makes it so easy to write insecure code that novices are pretty much guaranteed to produce vulnerabilities and even experts have to be vigilant to avoid accidentally stepping on one of the many landmines in the language.

If the designers of the language (or framework) put a landmine in it, and a developer steps on it, the designers of the language absolutely bear some responsibility for the fact that the landmine was there to be stepped on.

Re: Github Is Classy

#54

Post by ryanbrunner »

jamesgeck0 wrote: post #44
raganwald wrote: post #5

Perhaps I am misunderstanding the situation. Am I correct that the so-called “vulnerability” is that if you do a mass assignment from an untrusted source—form parameters—to a model without whitelisting attributes via attr_accessible, an attacker can write values to arbitrary attributes? If so, this isn’t a security vulnerability in Rails. A new Rails application isn’t vulnerable by default. It’s a security vulnerabil…

I'm a just a fool with a simple Rails app and copy of _The Design of Everyday Things_, but this seems like a usability issue. The default scaffolding doesn't contain an attr_accessible property, so it's not visible to the user that there's something missing. There's also no feedback from Rails that your attributes are insecure; only feedback if one tries to change an attribute explicitly made non-accessible. Figuring…

On top of that, Rails sometimes seems like it goes out of it's way to make attr_accessible a pain to use. I know more than once on my team someone has spent more than a few minutes trying to figure out why a model wasn't updating before they remembered to add a property to attr_accessible. In one case, we had a (rarely-used) field uneditable for a few weeks due to it not being included on the list.

I really liked wycats proposal of moving this sort of thing to the controller - it's a much more logical place to look, and I do think there's a thing as a too-skinny controller - mapping inputs to model attributes is a perfectly sane thing for the controller to do IMO.

Re: Github Is Classy

#55

Post by ryanbrunner »

thenduks wrote: post #13
skeletonjelly wrote: post #8

Earlier quoted context omitted.

Sure. But I feel like it's the job of a framework to hide away complexities to help the developer write code faster. I don't use Ruby/Rails but from my experience with ASP.NET MVC it seems that these helper functions exist for a reason. So why provide a tool that's inherently insecure? Caveat emptor?

On one hand you have `update_attributes` - IMO you should just never use this (seems raganwald agrees), but hey, it exists and it can be useful for simple stuff. So on the other hand is `attr_accessible` - You need to specify what is allowed to be mass-assigned if you want to use mass-assignment. This stuff is covered in the basic Rails Guides ( http://guides.rubyonrails.org/security.html#mass-assignment ). Saying th…

Pretty much every tutorial I can find on using controllers in Rails uses mass assignment, and the majority of them (including the beginners guide on guides.rubyonrails.org) don't mention attr_accessible or the security risk in allowing unrestricted mass assignment.

We're telling people to RTFM, but the FM doesn't say a thing about how this is dangerous.

If it was considered standard to avoid use of your brakes at all costs, and accelerate at maximum speed at all times, and this was reflected in the owner's manual - sure, I would think cars were unsafe.

Re: Github Is Classy

#56

Post by ryanbrunner »

xentronium wrote: post #50
kenneth_reitz wrote: post #3

The amount of bad press GitHub has received from this has blown me away. If anyone should be getting flamed for this, it should be the Rails maintainers that weren't open to the discussion.

> If anyone should be getting flamed for this, it should be the Rails maintainers that weren't open to the discussion. A bad workman always blames his tools.

If someone made a mitre saw that didn't have a guard over the blade, and people were getting their fingers cut off, I'd absolutely blame the tool.

Re: Github Is Classy

#57

Post by aneth »

raganwald wrote: post #5

Perhaps I am misunderstanding the situation. Am I correct that the so-called “vulnerability” is that if you do a mass assignment from an untrusted source—form parameters—to a model without whitelisting attributes via attr_accessible, an attacker can write values to arbitrary attributes? If so, this isn’t a security vulnerability in Rails. A new Rails application isn’t vulnerable by default. It’s a security vulnerabil…

Totally agree. The problem is the idiom of using mass assignment, which IMO should almost never be used.

This is not a bug or a security hole in Rails, but an issue with programmers not paying attention.

If Rails had no attr_accessible feature, it would be standard practice to always filter attributes in the controller, and no one would call this a rails issue - they would put the blame where it belongs: Github.

Instead, because attr_accessible exists, people are flaming that it should be enabled by default.

Re: Github Is Classy

#58

Post by SideburnsOfDoom »

xentronium wrote: post #50
kenneth_reitz wrote: post #3

The amount of bad press GitHub has received from this has blown me away. If anyone should be getting flamed for this, it should be the Rails maintainers that weren't open to the discussion.

> If anyone should be getting flamed for this, it should be the Rails maintainers that weren't open to the discussion. A bad workman always blames his tools.

> A bad workman always blames his tools.

So do expert workmen when given crappy tools.

Re: Github Is Classy

#59

Post by raganwald »

timr wrote: post #42
raganwald wrote: post #17

Earlier quoted context omitted.

so maybe some novice developers write an app using the generators and scaffolding baked into Rails, and as a result they get a security vulnerability that a more experienced developer might have avoided by doing extra work, and you say that's not a security problem in Rails? Young man, do not take that flippant tone with me (raps ruler on desk). Those novice developers failed to RTFM. If you want a framework that pro…

"Young man, do not take that flippant tone with me (raps ruler on desk). Those novice developers failed to RTFM." I suppose that the github developers all failed to RTFM, too, right? So either they're all n00bs, or we can safely assume that everyone makes mistakes when the framework makes mistakes easy to make. (Also, I suspect that you're joking about the "young man" thing, but it's probably worth pointing out that…

Of course I was joking about the young man thing. As I suspected you were with the “Gee” thing. It’s not a formal debate, a little “local” colour makes comments like this more enjoyable to read.

Re: Github Is Classy

#60

Post by ryanbrunner »

javascriptlol wrote: post #40
raganwald wrote: post #17

Earlier quoted context omitted.

so maybe some novice developers write an app using the generators and scaffolding baked into Rails, and as a result they get a security vulnerability that a more experienced developer might have avoided by doing extra work, and you say that's not a security problem in Rails? Young man, do not take that flippant tone with me (raps ruler on desk). Those novice developers failed to RTFM. If you want a framework that pro…

raganwald likes to put all the blame on "novices" and tell people to "RTFM" then take umbrage to the suggestion that he's defending the existence of bad design based on an idiotic, legalistic approach to conversation.

Especially ironic considering the thing that sparked this whole discussion was a vunerability on GitHub, of all places. If the developers at GitHub are lumped in with the uneducated "novices", who exactly are the experts?
Post reply on HN