71–80 of 295 posts
I installed this last night and the update came in at 15MB. Surely a one line bug shouldn't cause a 15MB update? Or is it that some things in iOS might be statically linked and those had to be pushed out as well?
seriously, tho, it is probably the whole binary this was part of.
Earlier quoted context omitted.
Regarding Adam Langley's comment about -Wall in gcc: -Wextra is what you want to have many more tests analysing your code. I wonder if this would have caught it.
A quick test shows neither -Wall nor -Wextra report anything on either gcc or clang. However, clang's -Weverything does complain (it implies -Wunreachable-code). Confusingly, gcc accepts -Wunreachable-code as a valid option, but then proceeds not to warn anything. (edit): Which is a known bug apparently, http://gcc.gnu.org/ml/gcc-help/2011-05/msg00360.html
I think what this really shows is that Apple has no unit tests to cover the security of their products. And that is pretty scary.
The problem is Apple (intentionally) understaffs and overworks, so I doubt they have the spare people to look at most commits.
Earlier quoted context omitted.
and goto statements everywhere :S
the gotos actually make sense in this case. unless you'd prefer some insane tree of if/else?
While Go has goto for tricky situations like this, because it has defer you don't have to use it often, assuming the free calls were needed (and the vars were not going to be GC'd):
defer SSLFreeBuffer(&hashCtx)
defer SSLFreeBuffer(&signedHashes)
if err = SSLHashSHA1.update(&hashCtx, &serverRandom); err != nil {
return err;
} else if err = SSLHashSHA1.update(&hashCtx, &signedParams); err != nil {
return err;
} else if err = SSLHashSHA1.final(&hashCtx, &hashOut); err != nil {
return err;
}
return nil;
}Earlier quoted context omitted.
A quick test shows neither -Wall nor -Wextra report anything on either gcc or clang. However, clang's -Weverything does complain (it implies -Wunreachable-code). Confusingly, gcc accepts -Wunreachable-code as a valid option, but then proceeds not to warn anything. (edit): Which is a known bug apparently, http://gcc.gnu.org/ml/gcc-help/2011-05/msg00360.html
The real WTF is those Warn flags, then. In what sane world does "extra" mean more than "all", and "all" and "everything" mean different things?
If blocks without curly braces ಠ_ಠ
I just made this - it'll tell you if you're vulnerable. https://gotofail.com/ Not very well tested, please let me know if it works for you. If you're on OS X Mavericks or on iOS 7 and haven't patched you should get big scary red text. Edit: posted here https://news.ycombinator.com/item?id=7282164
Safari/iOS 4.3.3: not vulnerable Safari/i0S 7.0.4: VULNERABLE Chrome/iOS 7.0.4: not vulnerable Looks like using Chrome instead of Safari may help; I'd say it would be more interesting if standard mail client can be fooled.
Worth noting that static analysis finds bugs like this immediately. TLS code seems like the perfect candidate to run through static analysis on every checkin. There are products such as Coverity and PVS-Studio that would have immediately flagged this and probably some open-source ones built around LLVM as well (unsure about this one, though). I personally use Coverity and have it hooked up in the same way everyone co…
There is no excuse to bypass static analysis nowadays. At very least should be part of the continuous integration build. The problem are the many developers that still think they are perfect and know the full C standard, including undefined parts.
Static analysis, regression tests and turning on all the warnings you possibly can should be mandatory, especially for such critical pieces of code.
How is this code not covered by a unit test? I'll admit I don't think of myself as great at unit testing, but the first thing I do when writing one for a new tool or class is use a code coverage tool to look for uncovered lines and once I have written a few basic behavioral tests I write tests to exercise and validate the output of the uncovered lines. This ensures that the tests I write to cover the public API don't…
This shows another of the benefits for the community of open sourcing code - because we can see exactly where and what the bug was steps can be taken in other projects to stop it happening there (I think Adam mentioned he was going to check for a test case in Chrome). If the code was closed all we would have is Apple's release note which just says validation steps were skipped...