On Friday, February 21st Apple published an update for iOS that fixed a serious security issue. What makes this issue interesting, is not only its severity but also the fact that the issue can be nicely pinned down two a single line of code. Conveniently, this code is open-source and available for analysis! In this post I’ll explain why this major security issue is, after all, the result of a number of quality issues, which are often undervalued as minor flaws.
Since Apple’s update on Friday the bug has been discussed on various blogs, websites and tweets. A good discussion of the bug can be found in Adam Langley’s blog. Interestingly, even mainstream media websites like the (German) Süddeutsche Zeitung and Der Spiegel try to explain this bug to their readers on the code level.
As the figure below shows, the bug is essentially caused by a superfluous goto fail; statement (code from Apple). This statement is not part of the if-block and, hence, always executed. Consequently, the following checks are not executed and the function will return a successful value, as err contains a successful value from the previous call to SSLHashSHA1.update().
Several violations of good coding practices can be nicely demonstrated using this bug:
Formatting: If an automatic code formatter would have been used, the odd role of the second goto fail; would have been a lot more obvious through the different indentation of the the two statements. This is also a problem in other places in the affected source file. For example, in the code shown below, one would expect the switch statement to be executed only if err is true. In fact, it does not have any relationship to the if statement. Also, the case statements that are syntactically on the same level have varying indents.
What is highly interesting about these aspects is that most of them are often considered issues that just affect software maintainability and not its functionality. Missing braces, bad formatting and naming, as well as the existence of goto statements, do not automatically make a system fail and are often considered minor flaws. The goto fail; example nicely illustrates how such a minor flaw (or a combination of many of them) can quickly turn into a major problem! Next time you are about to brush aside findings about the maintainability of your system, think twice.