Hacker News new | ask | show | jobs
by simias 4453 days ago

    if (1 + 2 + 16 > s->s3->rrec.length)
    if (1 + 2 + payload + 16 > s->s3->rrec.length)
Come on. At least use a macro or something.

This always makes me cringe during code reviews.

1 comments

Yep, and no braces after the 'if' statement in the patch. Even after the previous ssl vuln (I thought it was gnutls- struggling to find the relevant hn discussion) was caused by an omission of braces after the 'if' statement.
You are thinking of "goto fail", a bug in Apple's Security framework. I would not claim that was "caused" by a lack of braces: even having the braces, that bug--in addition to a wide class of similar bugs--is still quite possible, even if in a few models of how the bug was caused it becomes slightly less likely. The best place to lay blame for that kind of error is a stubborn insistence that error handling should involve boilerplate return value checks strewn throughout the code, with no attempt at abstraction or structure: it leads to numerous potential mistakes. Please read the various discussions attached to this article that made this claim:

https://news.ycombinator.com/item?id=7318039

Yes, that is exactly what I was thinking of. Thanks for the link. The discussion I was recalling is here

https://news.ycombinator.com/item?id=7282005

but the thread is a lot heftier now.