Hacker Newsnew | comments | ask | jobs | submitlogin
Ask HN: Has Hacker News been hacked/cracked?
105 points by code_devil 1861 days ago | comments
It seems like you can change the about field under PG's account http://news.ycombinator.org/user?id=pg using this appjet app http://notabank.appjet.net/


pg 1861 days ago | link

Yes. I made an unbelievably stupid mistake in the code that generates forms with labelled fields. It was basically functional programming taken a little too far: I generated the same form whether the fields were editable or not, and then later if there were no editable fields I just omitted the submit button. So anyone looking at the source of one of these pages could find a fnid that would work to modify the object displayed on it. (There's still a fnid, but it no longer does anything.)

-----

pg 1861 days ago | link

Actually on closer examination the bug was subtler than that. I did check whether the incoming request was allowed to modify each field it was trying to modify. But the code that decided whether a field was modifiable was using a four variable list to destructure on a five value description of the field. This caused the variable determining whether a field was modifiable to be bound instead to the value saying whether or not it should be displayed. Which meant every field that was displayed was modifiable.

-----

jganetsk 1860 days ago | link

For all people who say good programmers don't make type errors, here's a counterexample.

-----

pg 1860 days ago | link

It's an interesting question whether it's really a type error to mismatch the sizes of things. I feel like sometimes it is and sometimes it isn't.

-----

KayEss 1860 days ago | link

In a strongly typed language one would normally want to reify the number in the type system whenever possible, i.e. the use of (a,b,c,d,e) rather than [a,b,c,d,e] in Haskell (that's a 5-tuple rather than a 5 element list).

I think that people used to dynamic programming languages would normally pick the list rather than the tuple simply because it at first appears easier. I think Dijkstra had something to say on that particular subject :)

-----

jganetsk 1860 days ago | link

Agreed. But it sounded like, in this case, you were using a list to represent that which would have been represented by a record in a statically typed language.

I'm imagining you'd have some kind of Field record type, with isDisplayable and isModifyable fields. Instead of destructuring as a list (or tuple), you would explicitly look for isDisplayable and isModifyable.

But, of course, you could still make the same mistake in a statically typed language if you chose not to set up the data structure this way.

-----

IHackedHN 1861 days ago | link

That's actually pretty interesting to me. I didn't notice that when I was looking at the Hacker News code. Perhaps Arc shouldn't allow a list to be destructured except with a list of the same length? Allowing differing lengths seems likely to cause subtle bugs like this.

-----

pg 1861 days ago | link

I generally tried to err on the side of flexibility, and it seemed more flexible not to require the lengths to match. I could imagine cases where you might want to use patterns both longer and shorter than the lists they were matched against. On the other hand, I'm not sure I've ever ended up taking advantage of this.

-----

IHackedHN 1861 days ago | link

Maybe you could use something along the lines of &rest to optionally support a pattern shorter than the matched list?

-----

pg 1861 days ago | link

You could do that with a dot now. The trouble is, you end up creating a junk variable:

    arc> (let (x . y) '(a b c) y)
    (b c)

-----

wingo 1861 days ago | link

You don't have to name it. It could be _.

    (let (x . _) '(a b c) x)
It sounds like you're treating list destructuring like CL's mvbind. While it's subjective of course, I'm more comfortable with the runtime doing what I mean when I control the number of values (modulo `(apply values ...)').

I'm not comfortable with destructuring values "flexibly" when I don't control the data coming in.

-----

parenthesis 1860 days ago | link

Except "_" is a perfectly good name for a symbol in Arc.

"nil" could be used instead, since nil isn't allowed to be rebound.

  (let (x . nil) '(a b c) x)
even works already, since (unless it's been changed in some newer version of Arc I haven't installed yet) Arc just silently ignores the let-ing of nil:

  (let nil t nil) ---> nil

-----

simonb 1861 days ago | link

But at the same time this junk variable is code-as-documentation professing the incomplete destructuring is intentional.

You could go a step further and have a special ignore symbol, (for instance * ) like most pattern matching languages/libraries have with the property that it can appear multiple times:

  arc> (let (x * y . *) '(a b c d e f) y)
  c

-----

IHackedHN 1861 days ago | link

Making users create junk variables seems to me like better behavior than the current implicit behavior. It's your decision, though, of course.

Anyway, is it safe to assume that you're not going to try to get me arrested or anything?

-----

pg 1861 days ago | link

I feel like unnecessary variables are extra bad; in cognitive terms they feel like they add more than one token to the length of a program.

Do you mean your username is actually descriptive? Since you are obviously a Lisp hacker, I'd be happy to have a truce. Can you send me an email?

-----

IHackedHN 1861 days ago | link

Thank you. I've sent you an email.

-----

dfranke 1861 days ago | link

For a language of my own design I'd agree, but I think that would be rather severely contrary to the spirit of Arc.

-----

unalone 1861 days ago | link

That would suggest, then, that the comment points and submission points are also controllable within some form, since they were tampered with also. Is that the case? (I'm not asking because I don't believe you - I'm just wondering how those numeric fields were altered.)

-----

pg 1861 days ago | link

Any field of any object could be changed this way.

-----

suhail 1861 days ago | link

Almost sounds like SQL Injection, perhaps no escaping on fnid?

-----

markbao 1861 days ago | link

Edit: I'm wrong, see below.

If I'm not mistaken, that had nothing to do with SQL injection. The fnid basically was the authenticator that allowed a person to edit a page, regardless of who was logged in.

What about other HN-powered Arc sites? Are they vulnerable as well? I won't name names, because I'm guessing they are indeed vulnerable.

Edit: Yes, they are.

-----

pg 1861 days ago | link

Not quite. A fnid is a hash key pointing to a closure on the server.

-----

markbao 1861 days ago | link

Ah, okay. Wrong guess. Everyone can downvote me now :)

-----

ralph 1860 days ago | link

With hindsight, do you still feel fnids are a good way to go? They present an obvious bottleneck in that the server has to remember many closures for a period of time as opposed to having each client remember their share. I know you drastically reduced the use of fnids some time ago. I wonder if the remainder are a vestige that's OK to live with because life's too short, or there's fundamentally a strong argument for them I'm missing.

-----

pg 1860 days ago | link

Sometimes these closures have more stuff in them than you could conveniently put in a url. For example, they might refer to another closure describing what to do next. When you try to do something that requires you to be logged in, for example, you're given the login page, with a fnid referring to a closure describing what you were in the middle of doing. That's why after logging in you're able to continue doing whatever you were doing.

-----

dha 1860 days ago | link

Agreed. How about making the fnid a signed cache of often used environment variables(instead of a random string), which could also be looked up via a more time consuming method if needed. Depends really on your traffic profile how much data could reasonably and cost-effectively be cached, but it could be a very sizable portion saved by retrofitting the current method.

-----

wlievens 1861 days ago | link

Indeed. fnid = function id

-----

ambition 1860 days ago | link

I think I've found and fixed for hnacademic.com, thanks to the wonderful details here.

-----

tptacek 1860 days ago | link

It's very much like the "mass assignment" bugs that hit Rails servers when people forget to limit the writeability of Active Record attributes.

-----

brlewis 1860 days ago | link

Escaping is needed when you pass strings to a program that parses and interprets them. That's not part of HN's architecture AFAICT. There shouldn't be any opportunity to introduce a vulnerability similar to SQL injection.

-----

inerte 1861 days ago | link

And are the YC application forms secure(d)?...

-----

pg 1861 days ago | link

Against this bug at least, yes.

-----

IHackedHN 1861 days ago | link

You fixed the hole with impressive speed. I've got to give you that.

Out of curiosity, why did you take the site down two times before applying the fix?

-----

pg 1861 days ago | link

The first time Trevor restarted it a few seconds before the fixed code actually arrived on the server.

-----

pelle 1861 days ago | link

I'm glad no major damage happened.

Anyway your mention of fnid prompted me to learn a bit about arc and the arc web server which looks very interesting. Might have to play with it a bit.

http://arcist.com/doc/srv.html

-----

markbao 1861 days ago | link

Whoever did this, please post a Tell HN or otherwise an article on how you did it. I'm sure others are curious (and would make a good starting point for patching Arc)

From the source, it looks like there was a vulnerability in which the fnid (I'm guessing a string that authenticates a user to edit an item?) was searched for on PG's profile page (using the regex /<input type=hidden name="fnid" value="([^"]+)">/. Then a POST request was made on the standard profile saving resource news.ycombinator.com/x, with the fnid which authenticated the user's permission to edit the page, along with the about text, as parameters.

Edit: PG says the fnid just points to a closure on the system. See above. Which means... all you needed was a randomly generated fnid, and that's all that you needed to edit anyone's page. Apparently?

Clever, or just poor authentication design. But that's only one half of the exploit. How were the points done? I'm going to rule out millions of accounts created.

-----

IHackedHN 1861 days ago | link

The points were editable through similar means. Pages like http://news.ycombinator.com/edit?id=519433 contained a form that allowed all of a story's details -- including title, url, and score -- to be edited. Just like with profiles, the story editing forms didn't have fields for non-admins, but just like with profiles, it was possible to submit a request with the fnid from the form regardless of admin status.

-----

unalone 1861 days ago | link

I'm going to disagree. Chances are it was XSS or - shudder - simple injection. It was a basic vulnerability and it got fixed. We don't need to know the details - though I'd like if whoever did this got banned for a while for being an obnoxious jerk.

(When you think about it, this was pretty simplistic. All anybody did was edit text fields on the site. That hints it was injection.

-----

joshuaxls 1861 days ago | link

Yes, it was hacked. Here's the original post from a lesser hack earlier today with pg's response containing the "not a bank" quote:

http://news.ycombinator.org/item?id=518752

-----

nx 1861 days ago | link

Okay, trust doesn't work as well as correct security measures. That's sad, but true.

-----

Spyckie 1861 days ago | link

http://source.notabank.appjet.net/

-----

kqr2 1861 days ago | link

Just in case the original app gets taken down, I copied the source to pastebin:

http://pastebin.com/f1a67398f

-----

pelle 1861 days ago | link

I caught it here:

http://skitch.com/pelle/beksu/hacker-news-hacked

-----

GeoJawDguJin 1861 days ago | link

There are a lot of bogus links showing up on the front page with obviously falsified vote counts (numbers starting with the digits "1337"). I'd say, yeah, it's been seriously compromised.

-----

jwb119 1861 days ago | link

Re: 1337 http://en.wikipedia.org/wiki/Leet

-----

The_Sponge 1859 days ago | link

Yes, that's the joke.

-----

erlanger 1861 days ago | link

We know.

-----

unalone 1861 days ago | link

Yes, it has been. Looks like the site had some major vulnerabilities.

I emailed PG, if he didn't know already, and slowly some of the things are being fixed back. PG's account is still vulnerable as of this posting. EDIT: No it's not.

-----

pmikal 1861 days ago | link

Certainly seems like it.

-----

sho 1861 days ago | link

Dodged a bullet there I'd say. At least your "hacker" seems to be a reasonable guy. Seems like he could have done a lot more damage. I hope your backups are up to date and verified recoverable; a more malicious intruder might not be so kind.

While I appreciate and admire the sentiment that this is a "community of trust", security still must be taken seriously. There are plenty of guys out there with the ability to pull such tricks; they may not care about trust, and the website is accessible to anyone, good or bad.

-----

chanux 1861 days ago | link

Of course it was hacked... By the creator of it :). That's why other hackers find it interesting.

For the question whether it's cracked... I dunno but nothing is perfect.

-----

chanux 1860 days ago | link

Looks like the hackers don't understand that Hacker News is a nice hack altogether.

-----

markokocic 1858 days ago | link

Nice and insucre one.

Anyways, the fact it is hacked drove some nice traffic to HN from reddit :)

-----




Lists | RSS | Bookmarklet | Guidelines | FAQ | DMCA | News News | Feature Requests | Bugs | Y Combinator | Apply | Library

Search: