`dtls_get_cookie`: dereference before the length check (currently masked by `dtls_create_cookie`)
メンテナーはふだん 2 日以内に返信
まだ誰も着手していません。
評価
調査の方向性
dtls_get_cookie 内の dtls.c:439 から始め、SKIP_VAR_FIELD の境界チェックを、その後に続く長さチェックと併せて確認します。提供された AddressSanitizer の再現手順を実行して境界外読み取りを確認し、追加された長さガードによって sanitizer エラーなしで同じアラートが返されることを検証します。
索引モデルが issue の本文から書いたものです。
説明
Not urgent — I believe this is unreachable in the current tree, and I say why
below. It is a hardening report: the check that makes the dereference safe lives
in a different function.
The site
dtls.c:439
msglen -= DTLS_HS_LENGTH + DTLS_CH_LENGTH;
msg += DTLS_HS_LENGTH + DTLS_CH_LENGTH;
/* skip session id */
SKIP_VAR_FIELD(msg, msglen, uint8, DTLS_ALERT_HANDSHAKE_FAILURE,
"get_cookie, session_id");
if (msglen < (*msg & 0xff) + sizeof(uint8))
return dtls_alert_fatal_create(DTLS_ALERT_HANDSHAKE_FAILURE);
SKIP_VAR_FIELD is correctly bounded but can leave msglen == 0, with msg
pointing one past the end of the message. The next line then reads *msg.
This site reads a length-prefixed field by hand, and it is doing the second
half of what SKIP_VAR_FIELD does while omitting the first:
#define SKIP_VAR_FIELD(P, L, T, A, M) { \
size_t skip_length = sizeof(T); \
if (L < skip_length) { ... return ...; } \ /* <-- this one */
skip_length += dtls_ ## T ## _to_int(P); \
if (L < skip_length) { ... return ...; } \ /* <-- present */
Reproduction
dtls_get_cookie is static, so the driver includes the translation unit.
Clean clone of master (3482499), the project's own CMake build with
-fsanitize=address added and nothing else changed:
git clone --depth 1 https://github.com/eclipse-tinydtls/tinydtls.git td && cd td
cmake -S . -B asan -DCMAKE_C_FLAGS="-fsanitize=address -g -O0" \
-DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address"
cmake --build asan -j4
direct.c:
#include <stdlib.h>
#include <stdio.h>
#include "dtls.c"
int main(void) {
size_t msglen = 60;
uint8 *msg = malloc(msglen); /* exactly msglen bytes */
memset(msg, 0, msglen);
msg[12] = 0xfe; msg[13] = 0xfd; /* version, at msg + DTLS_HS_LENGTH */
msg[46] = 13; /* session_id length; body ends at 60 */
uint8 *cookie = NULL;
printf("dtls_get_cookie -> %d\n", dtls_get_cookie(msg, msglen, &cookie));
free(msg); return 0;
}
gcc -fsanitize=address -g -O0 -I. -Iasan -o direct direct.c asan/libtinydtls.a && ./direct
==251536==ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 1 at 0x50600000005c
#0 dtls_get_cookie dtls.c:455
0x50600000005c is located 0 bytes to the right of 60-byte region
The message is a 60-byte handshake message whose session id ends exactly at the
end of the buffer: 12 (handshake header) + 34 (dtls_client_hello_t) + 1
(session id length = 13) + 13 = 60.
Why I think it cannot happen today
The only caller is dtls_0_verify_peer (dtls.c:2249), which calls
dtls_create_cookie first with the same (data, data_length) and returns on
error. dtls_create_cookie performs the longer parse — after the session id it
also skips the cookie, the cipher suites and the compression list — and its
cookie skip requires at least one byte to remain. That is the byte
dtls_get_cookie dereferences unchecked, so no message accepted by
dtls_create_cookie can drive dtls_get_cookie out of bounds.
I checked this with a bounded model checker rather than only by reading:
composing the two calls the way dtls_0_verify_peer does, and running every
message up to 60 bytes with all bytes unconstrained, reports no violation — and a
reachability probe confirms messages do reach dtls_get_cookie, so the result is
not vacuous. Caveats: bounded at 60 bytes, and the HMAC digest was replaced by
its documented length (the cookie's contents cannot affect whether
dtls_get_cookie is reached).
Why it still seems worth a line
- The invariant is non-local and undocumented: nothing in
dtls_get_cookie
says a caller must have rundtls_create_cookiefirst. A change to the cookie
path, or a second caller, makes it live. - A unit-level fuzz target on
dtls_get_cookie— a natural thing to add, since
the repository currently has none — hits it immediately.
Fix
if (msglen < sizeof(uint8) || msglen < (*msg & 0xff) + sizeof(uint8))
Behaviour-preserving for every input. When msglen == 0 the original
condition 0 < (*msg & 0xff) + sizeof(uint8) is true whatever the out-of-bounds
byte happens to be, since the right-hand side is at least 1; so both versions
take the same branch and return the same alert. When msglen >= 1 the added
disjunct is false and the two conditions are identical.
Confirmed: with the change, the reproduction above prints
dtls_get_cookie -> -552 — the same value as before — and ASan is silent.
- 主要言語
- C
- スター
- 122
- フォーク
- 67
- 平均マージ
- 6日 8時間
- マージ済み PR(30日)
- 5
環境構築
- Dockerfile・Docker Compose ファイルなし
- プルリクエストのテンプレートなし
- コントリビューションガイドを読む
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
eclipse-tinydtls/tinydtls のほかの issue
-
ECDHE_ECDSA ClientKeyExchange rejected as too short when DTLS_PSK_MAX_CLIENT_IDENTITY_LEN > 64対応中かも このイシューにリンクされたプルリクエストがオープン中、またはマージ済みです。 オープンbug good first issue please retest
難易度 2/5 1〜3時間 初心者へのやさしさ 76/100
eclipse-tinydtls/tinydtls#298 · コメント 2 件 ·
メンテナーはふだん 2 日以内に返信
-
MSVC build is broken (undeclared `DTLS_DEBUG_BUF_SIZE`)対応中かも @mrdeep1 が 10 日前に担当しました。 オープン
難易度 2/5 1〜3時間 初心者へのやさしさ 25/100
eclipse-tinydtls/tinydtls#283 · コメント 5 件 ·
メンテナーはふだん 2 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 57/100
eclipse-tinydtls/tinydtls#278 · コメント 3 件 · リアクション 1 件 ·
メンテナーはふだん 2 日以内に返信
-
難易度 4/5 3〜5日 初心者へのやさしさ 45/100
eclipse-tinydtls/tinydtls#274 · コメント 2 件 ·
メンテナーはふだん 2 日以内に返信
-
bug
難易度 4/5 3〜5日 初心者へのやさしさ 48/100
eclipse-tinydtls/tinydtls#273 · コメント 2 件 ·
メンテナーはふだん 2 日以内に返信
eclipse-tinydtls/tinydtls の issue をすべて見る
似ている issue
-
enhancement
難易度 1/5 1時間未満 初心者へのやさしさ 85/100
FujiNetWIFI/fujinet-lib#122 · コメント 1 件 ·
メンテナーはふだん 1 日以内に返信
-
bug format I/O medium priority rt_lib
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
メンテナーはふだん 1 日以内に返信
-
bug
難易度 2/5 1〜3時間 初心者へのやさしさ 62/100
compiler-explorer/compiler-explorer#9272 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 76/100
semantic-reasoning/wirelog#2163 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
メンテナーはふだん 3 日以内に返信