Hacktoberfest 2026:メンテナが10月に向けて印を付けた、オープンで初心者向けの issue。 Hacktoberfest の issue を見る

`dtls_get_cookie`: dereference before the length check (currently masked by `dtls_create_cookie`)

オープン 初心者向け
#279 コメント 2 件 リアクション 1 件 担当者 0 名 GitHub で見る

メンテナーはふだん 2 日以内に返信

まだ誰も着手していません。

評価

難易度
2/5
見積もり時間
1〜3時間
初心者へのやさしさ
68/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
静か
技術スタック
c
領域
security

調査の方向性

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 run dtls_create_cookie first. 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

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

eclipse-tinydtls/tinydtls のほかの issue

eclipse-tinydtls/tinydtls の issue をすべて見る

似ている issue

C の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。