💀 ♊️ 👌🏿 OpenSSLのチェックに関する退屈な記事 🏙️ 🏿 ▪️

少し前まで、OpenSSLに脆弱な人しか話せない脆弱性が発見されました。 PVS-Studioは、この脆弱性につながるエラーを見つけることができないことを知っています。したがって、OpenSSLに関する記事を書く理由はないと判断しました。最近では、このトピックに関する記事が多すぎます。ただし、PVS-Studioでこのエラーを検出できるかどうかを尋ねる手紙が大量に届きました。私はあきらめてこの記事を書きました。

OpenSSLを検証する

OpenSSLに深刻な脆弱性が発見されたことは誰もが既に知っていると思います。それでも、誰かがこれを見逃したり、詳細を知りたい場合は、このトピックに関するいくつかの記事に精通することを提案します。

簡単に言うと、他の人のデータへのアクセスを許可する脆弱性は、約2年間コードに存在していました。この間に、単一のコードアナライザーがそれを見つけたわけではありませんが、怠け者だけがこのライブラリをテストしませんでした。

OpenSSLもテストしました。このトピックに関するメモは次のとおりです。「 OpenSSLについて少し説明します。」私たちが見つけたものですが、それは深刻なものではないようです。現在、これらのエラーは修正されています。だから無駄にチェックされていません。

すでにHeartbleedのバグがある場合にOpenSSLをチェックするかどうかは指定しませんでした。いずれにせよ、PVS-Studioはそのようなバグを検出できないことを知っています。一般的に検出することは困難です。 OpenSSLプロジェクトはさまざまなツールでチェックおよびチェックされましたが、いずれのツールでもこのエラーは見つかりませんでした。たとえば、Coverity Scanコードアナライザーのリーダーである間違いは見つかりませんでした。これに関する注記：「 Heartbleed and Static Analysis 」、「 Heartbleed and Static Analysis （2）」。

実際、静的分析を使用してこのようなエラーを見つけることは非常に困難です。コードがわかりにくいです。メモリに保存されている値を考慮する必要があり、明示的な型変換の背後に隠されているものを理解する必要があります。人であっても問題が何であるかを理解することは困難です。静的アナライザーはここを通過します。これは静的分析方法論の欠陥ではありません。間違いは本当に複雑だというだけです。おそらく、そのような構造を検索するために事前にトレーニングしないと、そのような欠陥を見つけることができるツールはありません。

ソフトウェアブックマークを検索するように設計された既知および未知の静的解析ツールがまだあることに注意してください。おそらくそのようなツールは脆弱性を見つける可能性がありますが、私はそれを強く疑います。見つかった場合は、アナライザーの広告として使用します。もちろん、特別なサービス内で開発されたいくつかのツールがこの脆弱性を見つけるオプションがありますが、彼らはそれについて特に話しません。しかし、陰謀論はここから始まるので、それについては話しましょう。

私の個人的な意見。これは単なる間違いであり、ブックマークではありません。静的分析ツールは複雑であるため、それを検出する方法を知りません。以上です。

これで記事を終了できますが、そうではなく、まったく面白くないでしょう。そこで、 PVS-Studioで OpenSSLを再度チェックしました。特別なものは見つかりませんでしたが、それでもコードのいくつかのセクションを見てみましょう。

なぜそんなに少ないのですか？はい、OpenSSLは質の高いプロジェクトだからです。深刻な脆弱性が見つかったからといって、コードがひどいというわけではありません。多くのアプリケーションでは、はるかに大きな穴があり、誰もそれを必要としないと思います。さらに、OpenSSLプロジェクトは多くのツールでテストされています。

分析結果

繰り返しますが、特定のエラーは見つかりませんでした。以下のテキストをエラーの説明としてではなく、不正確に思えたコードに対するコメントとして考えるとよいでしょう。ハエから象を膨らませているというコメントは後で見たくありません。

疑わしい比較

typedef struct ok_struct { .... size_t buf_len_save; size_t buf_off_save; .... } BIO_OK_CTX; static int ok_read(BIO *b, char *out, int outl) { .... BIO_OK_CTX *ctx; .... /* copy start of the next block into proper place */ if(ctx->buf_len_save - ctx->buf_off_save > 0) .... }

PVS-Studio警告：V555「A-B> 0」という表現は「A！= B」として機能します。 bio_ok.c 243

式（ctx-> buf_len_save-ctx-> buf_off_save> 0）は、一見思われるかもしれませんが機能しません。

ここで、彼らは条件をチェックしたいようです（ctx-> buf_len_save> ctx-> buf_off_save）。そうではありません。実際、比較される変数は符号なしの型です。 1つの符号なし変数を別の符号なし変数から減算すると、符号なしの型の結果が生成されます。

変数が一致しない場合、条件（ctx-> buf_len_save-ctx-> buf_off_save> 0）は常に満たされます。つまり、次の2つの式は同等です。

（ctx-> buf_len_save-ctx-> buf_off_save> 0）
（ctx-> buf_len_save！= ctx-> buf_off_save）

C言語にあまり詳しくない人のための説明。 経験豊富な開発者は読むことができません。

2つの32ビット符号なし変数があるとします。

符号なしA = 10;

符号なしB = 20;

条件（A-B> 0）が満たされているかどうかを確認します。

減算の結果（A-B）は、10u-20u = 0xFFFFFFF6u = 4294967286uです。

次に、符号なしの数値4294967286uをゼロと比較します。ゼロも符号なしの型にキャストされますが、それは問題ではありません。

式（4294967286u> 0u）は真です。

式（A-B> 0）は、A == Bの場合、1つの場合にのみ偽となります。

この比較は間違いですか？ プロジェクトデバイスに精通していないため、わかりません。間違いないと思います。

ほとんどの場合、これが事実です。変数 'buf_len_save'は通常、変数 '' buf_off_save 'よりも大きくなります。変数 'buf_off_save'の値が 'buf_len_save'に保存されている値に達する場合があります。この場合、変数が同じであり、このチェックが必要な場合。（buf_len_save <buf_off_save）のケースはおそらく不可能です。

トラブルを引き起こさない初期化されていない変数

初期化されていない変数を使用できる場所があります。しかし、それが悪い結果をもたらすことはありません。このコードは次のとおりです。

 int PEM_do_header(....) { int i,j,o,klen; .... if (o) o = EVP_DecryptUpdate(&ctx,data,&i,data,j); if (o) o = EVP_DecryptFinal_ex(&ctx,&(data[i]),&j); .... j+=i; if (!o) { PEMerr(PEM_F_PEM_DO_HEADER,PEM_R_BAD_DECRYPT); return(0); } .... }

PVS-Studio警告：V614潜在的に初期化されていない変数「i」が使用されました。 pem_lib.c 480

（o == false）の場合、変数 'i'は初期化されていない可能性があります。その結果、「j」に何が追加されるかは明確ではありません。（o == false）の場合、エラーハンドラーが起動し、関数が動作を停止するため、これは恐ろしくありません。

コードは正しいですが、正確ではありません。最初に変数「o」を確認し、その後で「i」を使用することをお勧めします。

 if (!o) { PEMerr(PEM_F_PEM_DO_HEADER,PEM_R_BAD_DECRYPT); return(0); } j+=i;

奇妙な課題

 #define SSL_TLSEXT_ERR_ALERT_FATAL 2 int ssl3_accept(SSL *s) { .... if (ret != SSL_ERROR_NONE) { ssl3_send_alert(s,SSL3_AL_FATAL,al); if (al != TLS1_AD_UNKNOWN_PSK_IDENTITY) SSLerr(SSL_F_SSL3_ACCEPT,SSL_R_CLIENTHELLO_TLSEXT); ret = SSL_TLSEXT_ERR_ALERT_FATAL; ret= -1; goto end; } .... }

PVS-Studio警告：V519「ret」変数には連続して2回値が割り当てられます。おそらくこれは間違いです。行を確認：376、377。s3_srvr.c 377

最初に、変数「ret」に値2が割り当てられ、次に-1が割り当てられます。おそらく、最初の割り当ては不要であり、偶然にコードに残っていたでしょう。

別のケース：

 int dtls1_retransmit_message(....) { .... /* save current state */ saved_state.enc_write_ctx = s->enc_write_ctx; saved_state.write_hash = s->write_hash; saved_state.compress = s->compress; saved_state.session = s->session; saved_state.epoch = s->d1->w_epoch; saved_state.epoch = s->d1->w_epoch; .... }

PVS-Studio警告：V519「saved_state.epoch」変数には、連続して2回値が割り当てられます。おそらくこれは間違いです。行を確認してください：1277、1278。d1_both.c 1278

潜在的なNULLポインターの逆参照

（私の経験では）プログラムで最もよくある失敗は、チェックされる前にポインターを逆参照することです。これは必ずしも間違いではありません。多くの場合、ポインターがnullになることはありません。ただし、これは潜在的に危険なコードです。特にプロジェクトが急速に変化する場合。

OpenSSLにはそのような間違いがあります。

 int SSL_shutdown(SSL *s) { if (s->handshake_func == 0) { SSLerr(SSL_F_SSL_SHUTDOWN, SSL_R_UNINITIALIZED); return -1; } if ((s != NULL) && !SSL_in_init(s)) return(s->method->ssl_shutdown(s)); else return(1); } .... }

PVS-Studio警告：V595 nullptrに対して検証される前に、 's'ポインターが使用されました。行を確認してください：1013、1019。ssl_lib.c 1013

最初は、ポインター「s」が使用されます：（s-> handshake_func == 0）。

そして、それだけがチェックされます：（s！= NULL）。

別のより複雑なケース：

 #define bn_wexpand(a,words) \ (((words) <= (a)->dmax)?(a):bn_expand2((a),(words))) static int ubsec_dh_generate_key(DH *dh) { .... if(bn_wexpand(pub_key, dh->p->top) == NULL) goto err; if(pub_key == NULL) goto err; .... }

PVS-Studio警告：V595 nullpubrに対して検証される前に、 'pub_key'ポインターが使用されました。行を確認：951、952。e_ubsec.c 951

エラーの場所を理解するには、マクロを展開する必要があります。次に、次のコードを取得します。

 if((((dh->p->top) <= (pub_key)->dmax)? (pub_key):bn_expand2((pub_key), (dh->p->top))) == ((void *)0)) goto err; if(pub_key == ((void *)0)) goto err;

ポインター「pub_key」に注意してください。

最初は逆参照されます：（pub_key）-> dmax。

以下では、ゼロと等しいかどうかがチェックされます：（pub_key ==（（void *）0））。

追加のチェック

変数が同じ値で2回比較されるコードがいくつかあります。これは間違いではないと思います。 2回目のチェックだけが偶然に書かれており、余分なものであるようです。削除できます。

追加チェックN1

 int ASN1_PRINTABLE_type(const unsigned char *s, int len) { .... if (!( ((c >= 'a') && (c <= 'z')) || ((c >= 'A') && (c <= 'Z')) || (c == ' ') || <<<<==== ((c >= '0') && (c <= '9')) || (c == ' ') || (c == '\'') || <<<<==== (c == '(') || (c == ')') || (c == '+') || (c == ',') || (c == '-') || (c == '.') || (c == '/') || (c == ':') || (c == '=') || (c == '?'))) ia5=1; .... }

PVS-Studio警告：V501「||」の左と右に同一のサブ式「（c ==」「）」があります演算子。 a_print.c 76

「<<<< ====」を使用して同一のチェックを強調表示しました。この重複チェックについては前の記事で書きましたが、修正されていません。したがって、これは間違いなく問題ではありません。

追加チェックN2、N3

 int ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek) { .... if ((type && (type != SSL3_RT_APPLICATION_DATA) && (type != SSL3_RT_HANDSHAKE) && type) || (peek && (type != SSL3_RT_APPLICATION_DATA))) .... }

PVS-Studio警告：V501「&&」演算子の左側と右側に同じ「式」の部分式があります。 s3_pkt.c 952

変数 'type'の値がゼロ以外であることが2回確認されます。

考慮されたコードフラグメントは別のファイルにコピーされるため、d1_pkt.c 760という不要な比較もあります。

行の長さが無効です

マジック定数を使用して文字列の長さを設定するのは良くありません。間違いを犯すのはとても簡単です。 OpenSSLでは、PVS-Studioアナライザーはこのような3つの場所に気付きました。

最初に失敗したマジックナンバー

これが間違いであることを示すために、BIO_write関数を呼び出すいくつかの例を見てみましょう。

BIO_write（bp、「エンコードのエラー\ n」、18）
BIO_write（bp、「\ n」、1）
BIO_write（bp、「：」、1）
BIO_write（bp、「：BAD OBJECT」、11）
BIO_write（bp、「Bad boolean \ n」、12）

これらの例からわかるように、最後の数字は文字列の長さを設定します。

そして今、間違ったコード：

 static int asn1_parse2(....) { .... if (BIO_write(bp,"BAD ENUMERATED",11) <= 0) goto end; .... }

PVS-Studio警告：V666関数「BIO_write」の3番目の引数を調べることを検討してください。値が、2番目の引数で渡された文字列の長さと一致しない可能性があります。 asn1_par.c 378

文字列「BAD ENUMERATED」の長さは11文字ではなく、14文字です。

失敗した2番目のマジックナンバー

 static int www_body(char *hostname, int s, unsigned char *context) { .... if ( ((www == 1) && (strncmp("GET ",buf,4) == 0)) || ((www == 2) && (strncmp("GET /stats ",buf,10) == 0))) .... }

PVS-Studio警告：V666関数 'strncmp'の3番目の引数を調べることを検討してください。値が、最初の引数で渡された文字列の長さと一致しない可能性があります。 s_server.c 2703

文字列「GET / stats」の長さは10文字ではなく、11文字です。最後のギャップは考慮されません。マイナーな欠陥ですが、それでも欠陥です。

失敗した3番目のマジックナンバー

 static int asn1_cb(const char *elem, int len, void *bitstr) { .... if (!strncmp(vstart, "ASCII", 5)) arg->format = ASN1_GEN_FORMAT_ASCII; else if (!strncmp(vstart, "UTF8", 4)) arg->format = ASN1_GEN_FORMAT_UTF8; else if (!strncmp(vstart, "HEX", 3)) arg->format = ASN1_GEN_FORMAT_HEX; else if (!strncmp(vstart, "BITLIST", 3)) arg->format = ASN1_GEN_FORMAT_BITLIST; else .... }

PVS-Studio警告：V666関数 'strncmp'の3番目の引数を調べることを検討してください。値が、2番目の引数で渡された文字列の長さと一致しない可能性があります。 asn1_gen.c 371

問題はここにあります：

 if (!strncmp(vstart, "BITLIST", 3))

文字列「BITLIST」の長さは7文字です。

少し気を取られた。読者は、PVS-Studioがこのようなエラーをどのように見つけるかを尋ねるかもしれません。説明します。関数呼び出し（この場合はstrncmp（））に関する情報を収集し、データマトリックスを作成します。

vstart、「ASCII」、5
vstart、「UTF8」、4
vstart、「HEX」、3
vstart、「BITLIST」、3

この関数には、文字列引数と数値引数があります。基本的に、文字列の長さは数字と同じです。したがって、この引数は文字列の長さを指定します。これは1つの場所に当てはまらないため、警告V666を発行する必要があります。

あまり良くない

「％08lX」を使用してポインタ値を出力するのは良くありません。これには「％p」があります。

 typedef struct mem_st { void *addr; .... } MEM; static void print_leak_doall_arg(const MEM *m, MEM_LEAK *l) { .... BIO_snprintf(bufp, BUF_REMAIN, "number=%d, address=%08lX\n", m->num,(unsigned long)m->addr); .... }

ポインターは関数に渡されませんが、型の値（unsigned long）が渡されます。したがって、コンパイラーと一部のアナライザーはサイレントのままになります。

PVS-Studioは、間接的にこの欠陥に気付きました。彼は、ポインターが（符号なしlong）型に明示的にキャストされるという事実を嫌います。これは間違っています。ポインタが「long」型に収まることを保証する人はいません。たとえば、これはWin64では実行できません。

正しい短いコードは次のとおりです。

 BIO_snprintf(bufp, BUF_REMAIN, "number=%d, address=%p\n", m->num, m->addr);

ポインター値が正しく印刷されない3つの場所があります。

mem_dbg.c 699
bio_cb.c 78
asn1_lib.c 467

おわりに

静的アナライザーはこのエラーを検出せず、非常に長い時間続きましたが、私はまだ誰もが日常業務で静的分析を使用することをお勧めします。すべての問題を解決し、プログラムコードをエラーから完全に保存する特効薬を探す必要はありません。最適な結果は、単体テスト、静的および動的分析、回帰テストなどの統合アプローチで達成されます。静的分析は、コーディング段階での多くのタイプミスや愚かなエラーを排除し、これにより、新しい機能やより徹底的なテストの作成など、他の有用なものの時間を節約します。

PVS-Studioコードアナライザーをお試しください。

この記事は英語です。

この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：Andrey Karpov。 OpenSSLプロジェクトのチェックに関する退屈な記事。

記事を読んで質問がありますか？

多くの場合、記事には同じ質問が寄せられます。ここでそれらに対する回答を収集しました： PVS-StudioおよびCppCatバージョン2014に関する記事の読者からの質問への回答。リストをご覧ください。

OpenSSLのチェックに関する退屈な記事