7 ms·

Don't trust comments

58 points by xx_ns 5y ago

SahAssar 5y ago
Comments state intent, code states reality. If those do not match one of them needs to change. When it comes to security no one should trust that intent matches reality.
longivitate 5y ago
Especially this one.
- capableweb 5y ago
  You're missing a `.. include :: ./arc-admin-credentials.json`
zatarc 5y ago
Don't trust this comment.
monetus 5y ago
4 days between notification and propagation of the fix. Thank you for poking around OP. Sanitizing your inputs isn't always simple.
- xx_ns 5y ago
  Aye, response was superb! It would've probably been less than 4 days had it not been on a weekend.
roylez 5y ago
Yes, rst is more powerful. So powerful that it can cause CVE.
nerdponx 5y ago
Can someone help me understand what actually happened here? As far as I can gather, the comment in the code wasn't a lie, but the overall system was complicated enough that there was a way for the thing-that-shouldn't-happen to happen anyway. So the docs weren't wrong, but there was a bug in the code that led to incorrect behavior that deviated from the docs. Or am I misunderstanding something?
- xx_ns 5y ago
  The comment did actually seem to be a flat out lie. It specifically states that include statements don't work when they did in fact work as normal. I don't think the lie was deliberate - the comment was probably erroneously added.
wahern 5y ago
Thus the hacker refrain, "lies, damned lies, and comments", for which oddly I'm unable to find any examples despite having seen it recited several times over the years. In fact, I believe I came across that rephrasing before learning of Twain's famous original. I always found it a more pithy justification for why source code comments should only explain why, not how or what.
xwdv 5y ago
You can definitely trust comments on Hackernews more than other sources such as Reddit.
- thisisbrians 5y ago
  Yes; poor comments tend to get downvoted.
spicybright 5y ago
I kind of wish there was a verifier for function/statement comments to flag inaccurate comments (besides interface descriptions + general comment at the top of a function.) Sort of a combination of a reverse github auto-pilot metric and checking how old a comment is based on it's surrounding code. You could even syntax highlight based on how accurate it thinks it is like how down voted HN comments fade out.
- xwdv 5y ago
  People don’t care about accurate comments, they care about comments they agree with.
  - tomcam 5y ago
    OK well you just rewrote my worldview. That observation can goes deep as one wishes go go with it...
    - xwdv 5y ago
      It becomes obvious this is the way the world works the more you start to generate unique comments rather than simply agree or upvote other people’s comments.
- Natsu 5y ago
  You could flag how many revisions it has been since the comments were updated in a function as compared to the code, but if that became a metric, someone would probably start optimizing for it by making trivial comment changes without doing real review.
throw10920 5y ago
Except you can't trust function names either. Remember the NSO iMessage exploit?[1] copyGifFromPath didn't really copy the GIF from the path. At some point, you do have to trust that the kernel API's documentation (or whatever) is correct[2], simply because it's physically impossible for you to exhaustively verify that each piece of software you use (transitively) has correct documentation and consistent semantics. That doesn't mean that you shouldn't audit third-party code, do code reviews, write tests, fuzz your code, and use static analysis and formal methods - in fact, you should do all of those things, if you can. But, "don't trust comments" is a gross oversimplification. Perhaps "trust, but verify" is a better pithy saying. [1] https://googleprojectzero.blogspot.com/2021/12/a-deep-dive-into-nso-zero-click.html https://googleprojectzero.blogspot.com/2021/12/a-deep-dive-i... [2] technically, if you did all of the above things, or found other people that did them, then you wouldn't have to trust documentation - but the vast majority of the time, most of the software you encounter will not have been thoroughly audited, tested, and fuzzed, with a nice formal specification
- jchw 5y ago
  On one hand, I agree. On the other, this does feel a bit whatabouty. Because yeah, you can’t check the kernel code, but you can and should check the code of direct dependencies. Not every time you do anything, but certainly whenever there’s something to be feared. Running code against untrusted inputs from the Internet is one of the few places that would justify “reverse engineer the kernel to make sure it’s safe” levels of concern, in the right conditions.
- ncmncm 5y ago
  "Trust, but verify" is a semantically null expression that appears to have filtered out of the Soviet Union during arms reduction negotiations, and eagerly taken up by Reagan administration appointees. You can only ever trust or verify. "Trust, but verify" is functionally identical to "verify", thus, equivalently, "distrust, therefore verify", with maybe a generous helping of cynicism, doublethink, and official mollification.
  - jcelerier 5y ago
nathias 5y ago
trust but verify
hinkley 5y ago
I don’t want comments, I want commentary. Every time I’m confidently wrong in a commit message - anything from the wrong bug ID to declaring victory prematurely - I wish I could go modify or amend it. Commits need to be in a separate version tree from their commit messages.

58 points by xx_ns 5y ago

Comments state intent, code states reality. If those do not match one of them needs to change. When it comes to security no one should trust that intent matches reality.

longivitate 5y ago

Especially this one.

capableweb 5y ago
You're missing a `.. include :: ./arc-admin-credentials.json`

zatarc 5y ago

Don't trust this comment.

monetus 5y ago

4 days between notification and propagation of the fix. Thank you for poking around OP. Sanitizing your inputs isn't always simple.

xx_ns 5y ago
Aye, response was superb! It would've probably been less than 4 days had it not been on a weekend.

roylez 5y ago

Yes, rst is more powerful. So powerful that it can cause CVE.

nerdponx 5y ago

Can someone help me understand what actually happened here? As far as I can gather, the comment in the code wasn't a lie, but the overall system was complicated enough that there was a way for the thing-that-shouldn't-happen to happen anyway. So the docs weren't wrong, but there was a bug in the code that led to incorrect behavior that deviated from the docs. Or am I misunderstanding something?

xx_ns 5y ago
The comment did actually seem to be a flat out lie. It specifically states that include statements don't work when they did in fact work as normal. I don't think the lie was deliberate - the comment was probably erroneously added.

wahern 5y ago

Thus the hacker refrain, "lies, damned lies, and comments", for which oddly I'm unable to find any examples despite having seen it recited several times over the years. In fact, I believe I came across that rephrasing before learning of Twain's famous original. I always found it a more pithy justification for why source code comments should only explain why, not how or what.

xwdv 5y ago

You can definitely trust comments on Hackernews more than other sources such as Reddit.

thisisbrians 5y ago
Yes; poor comments tend to get downvoted.

spicybright 5y ago

I kind of wish there was a verifier for function/statement comments to flag inaccurate comments (besides interface descriptions + general comment at the top of a function.) Sort of a combination of a reverse github auto-pilot metric and checking how old a comment is based on it's surrounding code. You could even syntax highlight based on how accurate it thinks it is like how down voted HN comments fade out.

xwdv 5y ago
People don’t care about accurate comments, they care about comments they agree with.
- tomcam 5y ago
  OK well you just rewrote my worldview. That observation can goes deep as one wishes go go with it...
  - xwdv 5y ago
    It becomes obvious this is the way the world works the more you start to generate unique comments rather than simply agree or upvote other people’s comments.
Natsu 5y ago
You could flag how many revisions it has been since the comments were updated in a function as compared to the code, but if that became a metric, someone would probably start optimizing for it by making trivial comment changes without doing real review.

throw10920 5y ago

Except you can't trust function names either. Remember the NSO iMessage exploit?[1] copyGifFromPath didn't really copy the GIF from the path. At some point, you do have to trust that the kernel API's documentation (or whatever) is correct[2], simply because it's physically impossible for you to exhaustively verify that each piece of software you use (transitively) has correct documentation and consistent semantics. That doesn't mean that you shouldn't audit third-party code, do code reviews, write tests, fuzz your code, and use static analysis and formal methods - in fact, you should do all of those things, if you can. But, "don't trust comments" is a gross oversimplification. Perhaps "trust, but verify" is a better pithy saying. [1] https://googleprojectzero.blogspot.com/2021/12/a-deep-dive-into-nso-zero-click.html https://googleprojectzero.blogspot.com/2021/12/a-deep-dive-i... [2] technically, if you did all of the above things, or found other people that did them, then you wouldn't have to trust documentation - but the vast majority of the time, most of the software you encounter will not have been thoroughly audited, tested, and fuzzed, with a nice formal specification

jchw 5y ago
On one hand, I agree. On the other, this does feel a bit whatabouty. Because yeah, you can’t check the kernel code, but you can and should check the code of direct dependencies. Not every time you do anything, but certainly whenever there’s something to be feared. Running code against untrusted inputs from the Internet is one of the few places that would justify “reverse engineer the kernel to make sure it’s safe” levels of concern, in the right conditions.
ncmncm 5y ago
"Trust, but verify" is a semantically null expression that appears to have filtered out of the Soviet Union during arms reduction negotiations, and eagerly taken up by Reagan administration appointees. You can only ever trust or verify. "Trust, but verify" is functionally identical to "verify", thus, equivalently, "distrust, therefore verify", with maybe a generous helping of cynicism, doublethink, and official mollification.
- jcelerier 5y ago

nathias 5y ago

trust but verify

hinkley 5y ago

I don’t want comments, I want commentary. Every time I’m confidently wrong in a commit message - anything from the wrong bug ID to declaring victory prematurely - I wish I could go modify or amend it. Commits need to be in a separate version tree from their commit messages.