パスワードを忘れた? アカウント作成
10704574 story
iOS

iOS7.0.6で修正された「最悪のセキュリティバグ」はありがちなコーディングミスで発生していた 172

ストーリー by hylom
お手本のようなコーディングミス 部門より
minet 曰く、

Appleが先日公開したiOS 7.0.6では、「過去最悪のセキュリティバグ」が修正されているという(アプリオ)。

SSL/TLSの不適切な扱いにより、データが暗号化されない場合があり、これにより通信内容が盗聴されるおそれがあるようだ。

現在利用中の環境にこのバグがあるかどうかは、gotofail.comというサイトで確認できる模様。

興味深いのが、このバグの要因とされているコードだ。SSLで使われる暗号鍵の検証を行うコードにバグがあったとのことなのだが、その途中で以下のように「goto」文が続いているミスがあったという(ImperialViolet - Apple's SSL/TLS bug)。

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;
}

コード全文は修正後のコードを見てほしいが、このように「goto fail;」が2行続いたことで途中の検証作業が飛ばされてしまい、その結果本来エラーとなるケースでエラーにならない、という問題が発生している模様。

この議論は賞味期限が切れたので、アーカイブ化されています。 新たにコメントを付けることはできません。
  • by Anonymous Coward on 2014年02月24日 18時26分 (#2550871)

    処理が1行だからといって、if文で格好つけるのはやめましょう。

  • テストしてねーの? (スコア:3, すばらしい洞察)

    by Anonymous Coward on 2014年02月24日 18時34分 (#2550879)

    OSのクセにこれぐらいのバグも検出できないぐらいテスト項目が粗い、ということがわかった。
    他の機能も危なそうだな。

    • by Anonymous Coward

      検証コードを書いた人も実行してないっぽいですね。

      /* FIXME: can this line be removed? */
      あるあるすぎる。

  • 証券会社は不適切なワンクリックで破産したりする
    https://www.google.co.jp/search?q= [google.co.jp]誤発注
    ジェイコムのより桁が2つ違う

    https://www.google.co.jp/search?q= [google.co.jp]俺はバグでこんなすごい被害を出したぞ

    も怖い

    • by Anonymous Coward

      過去最悪かどうかはわからないが、近年で見ると最悪レベルだと思う。

      つまり、ネットバンクやSNSなんかをSSLで使ってる人は多いと思うが、iOS/Macを使っている人の場合は、MITM攻撃で情報を盗み見できる状態にあったってことだ。
      iPhone/iPad/Macはローカルだけで使ってください、という話になるよ。

      これを最悪といわずに、なんという。

      だからAppleにしては珍しく、前倒しして早めに修正パッチをリリースしたわけで。

  • しっかり下まで落とせるわけだ。

  • >>コード全文は修正後のコードを見てほしいが

    これって件の問題が修正されたものなの?

  • by Anonymous Coward on 2014年02月24日 19時17分 (#2550921)
    goto関連のエラーを見るたびに申し訳なくなる私は後藤

    #本名ぶっぱなのでAC
  • 静的コード解析ツールを扱っている営業マンがApple本社に集まってきたりして。
    このパターンだと確実にツールに指摘されるところだが、Appleほどの会社がまだ使っていないのかなぁ?

  • 多分コピペが原因だろうね。
    経験上コピペがバグの原因になる可能性は高いから(俺も含めて)みんな気をつけよう!

    コピペしたら中身の修正漏れはないか、余計に(または少なく)行をコピってないか
    そもそもコピペをせずに関数化するかを慎重に確認しないとね(俺が…)。

  • by Anonymous Coward on 2014年02月24日 18時25分 (#2550870)

    警告を無視しちゃいかんという教訓を得たと。

    こんな感じに、if分を一行にまとめてしまえば、こんな不具合出なかった気がします。

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

    一行が長くなるのを嫌ったのかな?

    • by Anonymous Coward on 2014年02月24日 18時47分 (#2550897)

      一行にまとめるコーディングは嫌いですし、職業プログラマーはあまり使うべきではないとも思っています。
      なので対処はif分の処理が1行でも必ず括弧で括る事です。

      親コメント
    • unreachable codeの検出は、停止性問題と同じなんで、goto文みたいな自由度の高い構文ではうまく動かないんじゃないのかね。
      それ以前にgccもclangもgotoの警告はやる気ないから

      親コメント
    • by Anonymous Coward

      #define ErrorCheck(f) do { if ((err = f) != 0) goto fail; } while (0)

      ErrorCheck(SSLHashSHA1.update(&hashCtx, &serverRandom));
      ErrorCheck(SSLHashSHA1.update(&hashCtx, &signedParams));
      ErrorCheck(SSLHashSHA1.final(&hashCtx, &hashOut));

      ってやりたくなるんだけど、ダメ?

  • by Anonymous Coward on 2014年02月24日 18時32分 (#2550876)

    goto fail;で、全部エラーに飛ぶように見えるんだけど、違うの。
    goto successじゃないよね?

  • by Anonymous Coward on 2014年02月24日 18時34分 (#2550880)

    エラーにならんの?
    どういうこと?

    • by Anonymous Coward

      とよく読み直したらerrが0になっててそれをそのまま返してるからか。

typodupeerror

計算機科学者とは、壊れていないものを修理する人々のことである

読み込み中...