Naming and the Apple SSL Bug

There is a severe SSL/TLS Vulnerability in iOS and Mac OS X (that is at least fixed in iOS 7.0.6, although not yet in Mac OS X). For a good analysis, look at this article. One recommendation that I would make to improve the code is a simple matter of naming.

Let’s look at the code:

static OSStatus
SSLVerifySignedServerKeyExchange(SSLContext *ctx, bool isRsa, SSLBuffer signedParams,
                                 uint8_t *signature, UInt16 signatureLen)
{
    OSStatus        err;
    ...

    if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
        goto fail;
    if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
        goto fail;
        goto fail;
    if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
        goto fail;
    ...

fail:
    SSLFreeBuffer(&signedHashes);
    SSLFreeBuffer(&hashCtx);
    return err;
}

As you see, the code needs to ensure that SSLFreeBuffer is called on the signedHashes and hashCtx variables, hence they are using goto all over the place. Say what you want about goto, but in absence of a built-in try..finally (there is a Microsoft extension, but, well, that’s not standardized), this may be good – it’s certainly common to see goto in C code, and if someone knows a good alternative, that would be a benefit for many.

My concern is that there is a label called “fail” that is also used to return success (that is, whenever err is 0).This seems like a bug waiting to happen since it’s not actually failing (well, except that it’s failing to fail). One idea is to make sure that fail forces a failure state, use a second label for cleanup and jump over the failure in case of success:

if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
    goto fail;

err = sslRawVerify(.....);
if(err) {
    ...
    goto fail;
}

// If we get here without an error, we're golden!
goto cleanup;

fail:
    if(err == 0) err = ERR_SHOULD_NEVER_HAPPEN;
cleanup:
    SSLFreeBuffer(&signedHashes);
    SSLFreeBuffer(&hashCtx);
    return err;

This is still using a whole bunch of goto spaghetti, but it will make sure that fail actually fails, at least with some error state that says “The code did something it shouldn’t”.

There might also be a case to be made to assume that the function should always return a failure unless it’s proven the verification succeeded. Right now, the function does kinda the opposite: It leaves err uninitialized then then runs it through a gauntlet of checks that bail out once err is ever not equal to zero. Bailing out early is good, but I wonder if there should be a secondary variable success that is initialized to false and only set to true in the one spot we know there’s not an error.

That could be additional complexity without any real gain though. Personally, I try to write security-related code in a way where bugs tend to make valid cases fail rather than invalid cases pass (in other words: Give valid users too little access instead of invalid users too much), but that’s of course easier said than done.

In any case, I highly recommend calling variables, functions, labels after what they actually do, and in this case, fail is an inaccurate label.

What’s on the screen: Maniac Mansion

I’m currently looking into Commodore 64 game development again, and one of the interesting questions is “How to display the graphics?” The C64 has a plethora of display modes, and with the use of raster interrupts it is possible to mix the different modes to create crazy tricks. A while back, I asked a question titled “What exactly makes up a screen in a typical C64 game?” and got a good answer, but at the time I didn’t really understand it. In order to really understand how a C64 screen is made up, I’m looking at a few games and describe what exactly they do. The first game I’m looking at is Maniac Mansion, undoubtedly one of the most important adventure games ever written (Random fact: I was lucky enough to finally find an original C64 version of it :))

Anyway, let’s look at the first screen, the selection of kids:

This is purely done in multicolor character graphics:

Note that the program I use to rip character graphics – CharPad – makes it a bit hard for me to adjust colors, hence the palette is slightly off. Of note is Razor’s hair color, Michael’s Skin Color, Jeff/Sid/Wendy’s Hair or Earring. It seems that Light Blue (Color 14) is the common background, Black (Color 0) and Light Red (Color 10) are the common multicolor backgrounds, and the individual Color RAM color is Yellow (Color 7) for Maniac Mansion, Sid, Wendy and Jeff, White (Color 1) for START, Red (Color 2) for Razor and Brown (Color 9) for Michael. Notice that the drop shadow for the Maniac Mansion logo is the same as the common skin color.

The white border around the selected kids is also a character. The only sprite is the crosshair, which changes into a snail when in pause mode:

The game screen looks a lot more complex at first:

The Character is made from 4 different sprites:

The VIC-II can display up to 8 sprites per scanline, so I assume that a raster interrupt is used when all 3 kids are in the same screen.

Everything else is made up from character graphics (again, slightly off due to me not fixing up all the colors):

Looking at the screen ram (using VICE‘s sc command) shows this:

@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
ffffgfffffffghijjjjkjjjjlmgfffffffgfffff
@@@@g@@@@@@@gopqqqrstqqquvg@@@@@@@g@@@@@
|yz{gx|yz{bxg}~  !~s"  !"#gxxyz{bxgyz{xx
)*@@gx)*,@+xg}~-./~s"-./"#gx)*@@+xg*@@+x
6@@@gb6@@@7xg}~a8a~s"aaa"#gx6@@@7bg@@@7b
BCCCgbBCCCDbg}~aaEFsGHaa"#gbBCCCDbgCCCDb
6V@@ex6V@@7xW}~aaXYsZ[aa"#Wb6V@@7xWV@@7x
6@@@Ab6@@@7bA}~aaa~s"aaa"#Ab6@@@7bA@@@7b
ffffgfffffffg}~aaa~s"aaa"#gfffffffgfffff
WbWbWbWbWxWbI}~JJJ~s"JJJ"#IbWbWbWbWbWbWb
QbQbQbQbQbQRQSTTTTTUTTTTTVQWQbQbQbQbQbQb
fffffffffff.......IJKLMNO...ffffffffffff
1010_0_0 ...P............... .1010101010
010.I......P^.............@.ABCD01010101
.T IcccUV.P^...............@.AWXY__0 010
ccccccc\b]^.................@.b.cccccccc
cccccccc.......................ccccccccc
WALK TO@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
PUSH    OPEN   WALK TO  NEW KID TURN ON
PULL    CLOSE  PICK UP  UNLOCK  TURN OFF
GIVE    READ   WHAT IS  USE     FIX

Uhhh… Well, it’s a bit rough for sure, but it illustrates how the screen is composed. Another thing of note is that the C64 supports two different character sets (2×256 Characters) and that the second character set consists of a common font, including the inventory arrows. This is one of the reasons that there is no text in the main game screen – they use the second (font) character set for the title line, switch to the primary set to draw the screen, then back to the secondary set for the menu at the bottom.

There may be some further tricks in some areas (e.g, I haven’t checked how Chuck the Plant or the Radio in Dr. Fred’s room is done) but overall this is how the game works: Sprites for the characters and the crosshair/snail, two character sets for the game screen and the menu.