Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

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

Open Beginner friendly
  • Is something wrong?
#279 2 comments 1 reaction 0 assignees View on GitHub

Maintainers usually reply within 2 days

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
68/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
c
Domain
security

Research direction

Start at dtls.c:439 in dtls_get_cookie and inspect the SKIP_VAR_FIELD bounds checks alongside the following length check. Run the provided AddressSanitizer reproduction to confirm the out-of-bounds read, then verify the added length guard returns the same alert without the sanitizer error.

Written by the indexing model from the issue text.

Description

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.

Dominant language
C
Stars
122
Forks
67
Avg merge
6d 8h
Merged PRs (30d)
5

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from eclipse-tinydtls/tinydtls

All issues in eclipse-tinydtls/tinydtls

Similar issues

More C issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.