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

Tarsnap email confirmation bypass

daemonology.net

Post reply on HN
Print view View original

31–40 of 51 posts

Re: Tarsnap email confirmation bypass

#31

Post by paulannesley »

> That last part is ultimately the most important lesson from this: Comments matter!

In most cases, logically granular commits with good commit messages, and a knowledge of `git log` and `git blame` etc, is better than leaving comments. Comments can easily get out of sync with reality (see https://twitter.com/nzkoz/status/538892801941848064 ) and create a lot of noise that make reading the code harder (especially when the comment and code contradict each other).

I only leave brief comments where some code is necessary but at a glance doesn't look right, or has a non-obvious reason. But first I find a way to make it look right or be obvious.

Re: Tarsnap email confirmation bypass

#32

Post by MichaelGG »

paulannesley wrote: post #31

> That last part is ultimately the most important lesson from this: Comments matter! In most cases, logically granular commits with good commit messages, and a knowledge of `git log` and `git blame` etc, is better than leaving comments. Comments can easily get out of sync with reality (see https://twitter.com/nzkoz/status/538892801941848064 ) and create a lot of noise that make reading the code harder (especially whe…

Comments are helpful as an overview of a module, briefly outlining what the goals and major ideas. I'm currently starting to work on a large project, all in C, heavily manually threaded, with essentially zero comments. It's open source so I can't complain, but boy does it make figuring things out difficult. Especially in C, where so much code is pushing bytes and pointers around, that there could easily be unintended functionality that might not be desired.

Getting devs to write separate documentation is more difficult than comments, and is more likely to get out of sync. Better if they write some general overview inline. (On the opposite end, I also recently reviewed a project that has almost no comments, except on calls to malloc and free, with comments "get some memory" and "release memory".)

Re: Tarsnap email confirmation bypass

#33

Post by jonahx »

seagreen wrote: post #30
jonahx wrote: post #28

Earlier quoted context omitted.

> Code can only tell you about the implementation - never the intent. Maybe in some purely technical sense this is true, but in a meaningful one it isn't. At an absolute minimum, names reveal intent -- which is why naming is so important. Regarding your other example, you are always free to wrap what you don't control in objects that have the intention-revealing semantics (read: correctly named behavior) that you des…

Names are comments:) let thisIsAString = 1 But I agree with your overall point.

This is a great point, actually. And I have seen examples of it in code bases using Hungarian notation.

In practice, though, I think this kind of "lie" both happens and persists less frequently than comments that "lie" either by inaccuracy or obsolescence. People naturally have a lazier attitude toward the upkeep of comments ("they don't really affect the code") and somehow an inaccurate variable name seems more brazen than an incorrect comment.

Re: Tarsnap email confirmation bypass

#34

Post by MichaelGG »

jonahx wrote: post #28
zimpenfish wrote: post #25

Earlier quoted context omitted.

Code can only tell you about the implementation - never the intent. Taking an example from my code yesterday: $config->{template} = $container->template; There's not much can be clarified here, I don't think. But it tells you precisely nothing about why it's required in this instance. (There's no spec for the file format - all have the `template` key in the `container` section but some also have it in the `config` se…

> Code can only tell you about the implementation - never the intent. Maybe in some purely technical sense this is true, but in a meaningful one it isn't. At an absolute minimum, names reveal intent -- which is why naming is so important. Regarding your other example, you are always free to wrap what you don't control in objects that have the intention-revealing semantics (read: correctly named behavior) that you des…

Naming at the higher level is important (type names). But locals, eh, it's unlikeky that you can comprehend lines without the full context, as you say. And our working memory is limited. So might as well use 1- or 2-char names and keep the code less and thus more easily kept in-head. If this is confusing, there's probably too many locals, so setup new scopes (either by nesting or via separate functions).

Re: Tarsnap email confirmation bypass

#36

Post by sanderjd »

jballanc wrote: post #16

There was a paper a couple years ago out of Microsoft research (if I recall correctly), that looked at a number of vulnerabilities in OAuth as used by Facebook, Twitter, and a few others. The ah-hah moment for me, though, was that they identified these vulnerabilities by turning the usual view of a web app inside-out: instead of viewing the client/browser as one endpoint on a communications channel, they treated the…

I haven't seen that paper, but it strikes me as being exactly the right model. A lot of vulnerabilities on the web seem to arise from the tension between the goal to require as little persistent state as possible to serve requests, and the browser as a public channel that any non-persisted state must be sent across.

Re: Tarsnap email confirmation bypass

#37

Post by acdha »

caf wrote: post #8

The value of writing comments intended for your future self was confirmed in a strange way for me: I once found myself googling some faintly obscure question of systems programming, and soon found an article that answered my question perfectly. At that point I noticed with considerable surprise that I was reading a web archive of a Usenet posting I had made myself, some 10 years prior - of all the people to randomly…

In my case this is complicated by the fact that there's someone else with the same name who has both significant professional overlap and a similar writing style. There are a couple of times I've been half way through an archived mailing list post wondering how I managed to forget an answer so completely before scrolling down far enough to see his .sig at the end.

Re: Tarsnap email confirmation bypass

#38

Post by jonahx »

MichaelGG wrote: post #34
jonahx wrote: post #28

Earlier quoted context omitted.

> Code can only tell you about the implementation - never the intent. Maybe in some purely technical sense this is true, but in a meaningful one it isn't. At an absolute minimum, names reveal intent -- which is why naming is so important. Regarding your other example, you are always free to wrap what you don't control in objects that have the intention-revealing semantics (read: correctly named behavior) that you des…

Naming at the higher level is important (type names). But locals, eh, it's unlikeky that you can comprehend lines without the full context, as you say. And our working memory is limited. So might as well use 1- or 2-char names and keep the code less and thus more easily kept in-head. If this is confusing, there's probably too many locals, so setup new scopes (either by nesting or via separate functions).

I agree higher level names are far more important.

But I think clarity at the local level is nice, too. Let's say your function takes a name, sanitizes it, and then does some other processing, perhaps storing it. I think this makes the code more immediately clear than 2 character names:

    function storeName(name) {
        safeName = sanitize(name);
        // do other stuff that works with safeName
        //...
    }

Re: Tarsnap email confirmation bypass

#39

Post by bentcorner »

tome wrote: post #13
caf wrote: post #8

The value of writing comments intended for your future self was confirmed in a strange way for me: I once found myself googling some faintly obscure question of systems programming, and soon found an article that answered my question perfectly. At that point I noticed with considerable surprise that I was reading a web archive of a Usenet posting I had made myself, some 10 years prior - of all the people to randomly…

That's happened to me, but unfortunately I've mostly found my old questions, not my old answers!

I've had this happen to me a few times, but usually it's "I've answered this before for someone else, and now I have the same question. What was my answer again?"

Re: Tarsnap email confirmation bypass

#40

Post by strommen »

zimpenfish wrote: post #25
jonahx wrote: post #24

I love writeups like this, and enjoyed the level of detail Colin provided. I take away a different lesson, though: even simple web security is easy to get wrong, even for a very smart, very talented developer. I'm not sure what the solution is, though. As for the comments, while I don't take a hard line here, I agree with Bob Martin's quote: "Every time you write a comment, you should grimace and feel the failure of…

Code can only tell you about the implementation - never the intent. Taking an example from my code yesterday: $config->{template} = $container->template; There's not much can be clarified here, I don't think. But it tells you precisely nothing about why it's required in this instance. (There's no spec for the file format - all have the `template` key in the `container` section but some also have it in the `config` se…

There are definitely cases where comments are required to describe intent...the "Why?" of the code.

But the problem with comments is that they'll inevitably get out of sync with the code. And a wrong comment is far worse than no comment at all.

In a case like Colin's, I think something as simple as including "secure" or "secret" in the name of the variable would prevent this stuff from happening. If your variable is named `secureAccountCode` then it's unlikely you'll be silly enough to render it back to a hidden input (unless you're writing code comments for "Drunk Me" like the Disqus commenter on the article, in which case all bets are off).

There's a classic Joel on Software article about this, "Making Wrong Code Look Wrong" [1]

[1] http://www.joelonsoftware.com/articles/Wrong.html

Post reply on HN