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

Responsible Disclosure Policy

github.com

Post reply on HN
Print view View original

51–60 of 85 posts

Re: Responsible Disclosure Policy

#51

Post by einhverfr »

davesims wrote: post #38
georgemcbay wrote: post #31

Speaking as someone who isn't a Rails developer (but does use GitHub Enterprise for work projects), when this first broke I was on the side of github and thought homakov was acting irresponsibly. Now that more background is coming out, I think he probably did the Rails community at large a huge favor here. Had this just been fixed quietly on GitHub, that would certainly be better for GitHub's PR but the wider communi…

It's an easy mistake to make, but arguably no easier than, for instance, not escaping input strings to guard against SQL injection. IMO it falls to the developer to set protected on vulnerable attributes. This is pretty basic Rails security practice. EDIT: not 'escaping', but using hashes or formatted strings, etc., you get the idea.

I am glad you fixed this in your post btw, but I still think the initial thought of using escaping to solve issues like SQL injection shows the way we think about security is flawed.

In general we tend to see security flaws as programming flaws. In other words, the programmer makes a mistake and therefore there is a security hole. The problem with this approach is that programmers make mistakes all the time.

Certainly it is impossible to take all the weight off the programmer's shoulders. Mistakes will always allow a program to be misused, misdirected, and so forth. However, most security issues are best solved as architecture problems, not as developer problems.

For example input sanitation is generally a bad idea* beyond certain things we should never see in inputs. It's far better to find ways of making the input safe to the database and to the web interface that doesn't depend on it being sanitized on input.

* This is because you can only sanitize based on how you want things to go on output, whether you are outputting from your program to the db or to a user interface of some sort. If you sanitize for HTML, you can't use the same info reasonably well for LaTeX, etc.....

So I am of the considered opinion that data should be checked for basic sanity on input (no termination sequences in the middle of input strings, etc), and escaped on use or output. If you have a framework to do this, then you centralize that logic so you don't have to think about it every time. This drastically cuts down on things like XSS, SQL Injection, and the like.

This sort of thing again strikes me as something the framework should prevent. That's not necessarily a flaw of Rails if you use Rails as a toolkit for your own application frameworks. However, it is an architectural flaw, not a programming mistake.

Re: Responsible Disclosure Policy

#52

Post by petercooper »

MartinMond wrote: post #12
JumpCrisscross wrote: post #3

Given that: (1) the nature of the suspension was not communicated to Egor at the onset of the situation, nor, (2) noted in the blog post [1] describing how Github "detected the attack", I am inclined to believe that this is a response to the furious reaction to their suspension decision and was not, as this post implies, the game plan from the beginning. It's healthy that they've reversed their suspension but the lac…

I agree with you, but even if this was a reaction to public outcry the real reason to be disturbed is that top-notch Ruby devs like the GitHub guys didn't use attr_accessible. I can't wrap my mind around that. That you have to use attr_accessible is known throughout the Rails community since "ever". Only toy apps don't use it. It's like saving passwords in plaintext, only arguably even worse.

Or more specifically, since the public keys objects require an associated user, the old chestnut:

    @user.public_keys.build(...)
.. where @user is retrieved in a role based manner (that is, you only get the right @user if you are authorized to get it.)

Ultimately, this is less an issue of mass assignment specifically and more an overarching one of allowing a user to perform an action in the guise of another. But, of course, these mistakes are commonly made by developers of all skill levels! :-) (me included)

Re: Responsible Disclosure Policy

#53

Post by nknight »

prophetjohn wrote: post #44
georgemcbay wrote: post #31

Speaking as someone who isn't a Rails developer (but does use GitHub Enterprise for work projects), when this first broke I was on the side of github and thought homakov was acting irresponsibly. Now that more background is coming out, I think he probably did the Rails community at large a huge favor here. Had this just been fixed quietly on GitHub, that would certainly be better for GitHub's PR but the wider communi…

It's not like this is a little known pitfall with Rails. Anyone who has read Hartl's Rails tutorial knows about it[1]. It's very commonly mentioned in basics for Rails security. And I say all this someone who has never professionally developed for Rails. My experience with Rails consists of a couple half-done toy projects. I find it pretty surprising that Github makes this mistake. But I don't think they should be bu…

I'm frankly amazed at how optimistic HN seems to be about "professional" coding practices. To this day I find "professional" developers writing fresh SQL injection vulnerabilities with some frequency.

Re: Responsible Disclosure Policy

#54

Post by kragen »

The problem I see with this blog post is something I haven't seen mentioned in the comments. It's not GitHub's place to set policy on what kind of disclosure is or isn't "responsible". Egor Homakov's responsibility is not to GitHub; his responsibility is to other users. His moral duty upon finding a security vulnerability is to act in such a way that other users will be minimally hurt. It appears that he has fulfilled that responsibility spectacularly in this case.

GitHub has no business demanding his, or your, agreement to a legal contract that prohibits you from exercising your best judgment in such a case.

Furthermore, "responsible disclosure" is a propaganda euphemism for "allowing irresponsible vendors to cover their asses, possibly at the expense of their users". Terms like "responsible disclosure" have no place in a serious discussion. Please see the blog post by the Google security team at http://googleonlinesecurity.blogspot.com/2010/07/rebooting-r... for further details.

Re: Responsible Disclosure Policy

#55

Post by timcosgrove »

icebraining wrote: post #46
philwelch wrote: post #25

Earlier quoted context omitted.

Playing nice with a hacker who just broke into your service shouldn't take priority over: 1. Making sure he doesn't continue breaking into your service (by suspending his account) 2. Fixing the security flaw he used to break into your service 3. Appraising your users to the situation. I feel for the kid--he's just 18, and if he gets some good judgment to go along with his technical skill he'll go far. But I don't und…

How does suspending his account ensure he doesn't continue breaking into your service?

Because it's trivial for him to set up another account and break the service from there. His ability to do this exploit wasn't tied to his specific account. His point was that anyone could be doing this.

=edit= Sorry, I am apparently agreeing with the comment I replied to.

Re: Responsible Disclosure Policy

#57

Post by javascriptlol »

michaelbuckbee wrote: post #48
zzzeek wrote: post #39

Earlier quoted context omitted.

I'm not a Rails/Ruby user but any decent database abstraction layer or ORM should be using bound parameters for all literal values. "Escaping" of SQL strings is best left to the database driver.

You're correct and Rails does do this (handle parameters in such a way as to prevent SQL injection attacks), however it is always possible to circumvent these protections and code things up in such a way (concatenate your own raw SQL string and push it through) as to shoot yourself in the foot.

Or you could disallow raw SQL strings and always construct programmatically (e.g. building ASTs). All of these recurring holes are due to bad design, period. Imagine if your microwave manufacturer said "ultimately it's up to the consumer to avoid irradiating himself". Nobody expects you to be saved from sticking a drill into your face, but nor should it electrocute you by forgetting to do something.

Re: Responsible Disclosure Policy

#58

Post by javascriptlol »

prophetjohn wrote: post #44
georgemcbay wrote: post #31

Speaking as someone who isn't a Rails developer (but does use GitHub Enterprise for work projects), when this first broke I was on the side of github and thought homakov was acting irresponsibly. Now that more background is coming out, I think he probably did the Rails community at large a huge favor here. Had this just been fixed quietly on GitHub, that would certainly be better for GitHub's PR but the wider communi…

It's not like this is a little known pitfall with Rails. Anyone who has read Hartl's Rails tutorial knows about it[1]. It's very commonly mentioned in basics for Rails security. And I say all this someone who has never professionally developed for Rails. My experience with Rails consists of a couple half-done toy projects. I find it pretty surprising that Github makes this mistake. But I don't think they should be bu…

I don't find it in any way surprising that this has happened any more than I would find it surprising that if you put a big hole in a footpath that someone would fall into it.

Re: Responsible Disclosure Policy

#59

Post by why-el »

davyjones wrote: post #37
freehunter wrote: post #30

Earlier quoted context omitted.

Actually, the world _does_ stop turning because it's Sunday. Anything happening on the weekend is emergency management, which takes time to scramble. I work in network/information security, and if we had a security incident on a Sunday, it would go to our backup team (of one). If they (he) deemed it critical, they would notify the security director, who would notify my boss, who would notify the rest of the team. Thi…

> Actually, the world _does_ stop turning because it's Sunday. Incorrect. In Islamic countries, which are a part of this world, like Saudi Arabia and Oman have Thu-Fri weekend. http://en.wikipedia.org/wiki/Workweek_and_weekend#Islamic_co...

Not all Muslim countries do though. All north african countries have a saturday/sunday weekend, except Egypt. and even in Egypt some companies make sure a subset of their employees work on their weekend because, well, most of their partners are working.

Re: Responsible Disclosure Policy

#60

Post by reledi »

georgemcbay wrote: post #31

Speaking as someone who isn't a Rails developer (but does use GitHub Enterprise for work projects), when this first broke I was on the side of github and thought homakov was acting irresponsibly. Now that more background is coming out, I think he probably did the Rails community at large a huge favor here. Had this just been fixed quietly on GitHub, that would certainly be better for GitHub's PR but the wider communi…

I think everyone won in the end. Homakov got the credit he deserved, Rails' security flaw got a lot of attention, and GitHub had the chance to prove themselves, which in my opinion they did.
Post reply on HN