Skip to content

ZSTD_DCtx_refPrefix(): whether the prefix survives ZSTD_DCtx_reset(ZSTD_reset_session_only) after an error depends on where the frame failed #4843

Open

Description

@GeekQAQ

Describe the bug

zstd.h says a prefix referenced with ZSTD_DCtx_refPrefix() is used once:

Prefix is only used once. Reference is discarded at end of frame.
End of frame is reached when ZSTD_decompressStream() returns 0.

and that after an error the stream must be reset before reuse:

Note: when an operation returns with an error code, the @zds state may be left in undefined state. [...] In order to re-use such a state, it must be first reset, which can be done explicitly (ZSTD_DCtx_reset()) [...]

In practice, the prefix is consumed when a frame header is successfully decoded, not at the end of the frame. As a result, the same sequence (reference a prefix, the frame fails, ZSTD_DCtx_reset(dctx, ZSTD_reset_session_only), decode the next frame) behaves differently depending on where the failing frame stopped:

  • the frame header is rejected: the prefix is still referenced after the session reset, and the next frame is decoded with it, although nothing was referenced for that frame;
  • the frame fails in its first block: the prefix is gone after the reset;
  • the frame completes: the prefix is gone (as documented).

An application that treats the failed and reset frame as finished and releases the prefix buffer (which seems reasonable after a reset) then has the next frame read freed memory. Under ASan this is a heap-use-after-free in ZSTD_decompressSequencesLong_body.

ZSTD_reset_session_and_parameters and ZSTD_DCtx_refPrefix(dctx, NULL, 0) both drop the prefix in all cases.

To Reproduce

prefix_after_reset.c (below) compresses frame P against a prefix, so P cannot be decoded without it. For each case it references the prefix on a new DCtx, feeds a first input, resets the session, and then decodes P without referencing the prefix again.

cc -O1 -g -fsanitize=address,undefined prefix_after_reset.c -Ilib lib/libzstd.a -o prefix_after_reset
./prefix_after_reset
./prefix_after_reset free

Output on dev (49cf517, ZSTD_versionString() 1.6.0); v1.5.7 gives the same results:

fresh DCtx, no prefix, frame P:                         Data corruption detected
first frame OK,             reset session_only                 first: ok, 8192 bytes               then frame P: Data corruption detected
first frame bad block,      reset session_only                 first: Data corruption detected     then frame P: Data corruption detected
first frame bad header,     reset session_only                 first: Unsupported frame parameter  then frame P: ok, 8192 bytes
first frame bad header,     reset session_and_parameters       first: Unsupported frame parameter  then frame P: Data corruption detected
first frame bad header,     reset session_only + refPrefix(NULL) first: Unsupported frame parameter  then frame P: Data corruption detected

With free (the prefix buffer is freed after the session reset, bad-header case only):

ERROR: AddressSanitizer: heap-use-after-free on address ...
    #0 ... in __asan_memmove
    #1 ... in ZSTD_decompressSequencesLong_body lib/decompress/zstd_decompress_block.c
    #2 ... in ZSTD_decompressSequencesLong_bmi2 lib/decompress/zstd_decompress_block.c:2031
    #3 ... in ZSTD_decompressBlock_internal lib/decompress/zstd_decompress_block.c:2265
    #4 ... in ZSTD_decompressFrame lib/decompress/zstd_decompress.c:1017
prefix_after_reset.c
/* ZSTD_DCtx_refPrefix(): a prefix referenced for a frame whose header is rejected survives
 * ZSTD_DCtx_reset(ZSTD_reset_session_only) and is used by the next frame.
 *
 * Frame P is compressed against a prefix (it cannot be decoded without it).
 * Each case: reference the prefix, feed a first input, reset the session (as documented after an error),
 * then decode frame P WITHOUT referencing the prefix again.
 *
 *   cc -O1 -g -fsanitize=address,undefined prefix_after_reset.c -Ilib lib/libzstd.a -o prefix_after_reset
 *   ./prefix_after_reset          functional cases (no memory is freed)
 *   ./prefix_after_reset free     the application frees the prefix buffer after the reset (run under ASan)
 */
#define ZSTD_STATIC_LINKING_ONLY   /* only for ZSTD_frameHeaderSize(), used to build the corrupted test input */
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <zstd.h>

#define N 8192
static unsigned char src[N], comp[2 * N], out[8 * N];

static size_t decode(ZSTD_DCtx *d, const void *in, size_t n, size_t *outlen) {
    ZSTD_inBuffer ib = { in, n, 0 };
    ZSTD_outBuffer ob = { out, sizeof out, 0 };
    size_t r;
    do { r = ZSTD_decompressStream(d, &ob, &ib); } while (!ZSTD_isError(r) && r != 0 && ib.pos < ib.size);
    *outlen = ob.pos;
    return r;
}

static const char *res(size_t r, size_t outlen) {
    static char buf[2][64];
    static int k;
    if (ZSTD_isError(r)) return ZSTD_getErrorName(r);
    k ^= 1;
    snprintf(buf[k], sizeof buf[k], "ok, %zu bytes", outlen);
    return buf[k];
}

int main(int argc, char **argv) {
    int free_prefix = argc > 1 && !strcmp(argv[1], "free");
    unsigned char *prefix = malloc(N);
    unsigned long long x = 88172645463325252ULL;
    for (size_t i = 0; i < N; i++) { x ^= x << 13; x ^= x >> 7; x ^= x << 17; src[i] = prefix[i] = (unsigned char)x; }

    /* frame P: payload equal to the prefix, so it is one long match into the prefix */
    ZSTD_CCtx *c = ZSTD_createCCtx();
    ZSTD_CCtx_refPrefix(c, prefix, N);
    size_t n = ZSTD_compress2(c, comp, sizeof comp, src, N);
    ZSTD_freeCCtx(c);
    if (ZSTD_isError(n)) { printf("compress: %s\n", ZSTD_getErrorName(n)); return 1; }

    unsigned char bad_header[2 * N], bad_block[2 * N];
    memcpy(bad_header, comp, n);
    bad_header[4] |= 0x08;                          /* frame header descriptor: reserved bit -> rejected header */
    memcpy(bad_block, comp, n);
    bad_block[ZSTD_frameHeaderSize(comp, n)] |= 0x06; /* first block header: reserved block type */

    printf("zstd %s, frame P = %zu bytes\n", ZSTD_versionString(), n);
    {   ZSTD_DCtx *d = ZSTD_createDCtx(); size_t ol, r = decode(d, comp, n, &ol);
        printf("fresh DCtx, no prefix, frame P:                         %s\n", res(r, ol));
        ZSTD_freeDCtx(d); }

    struct { const char *name; const unsigned char *first; ZSTD_ResetDirective reset; int ref_null; } cases[] = {
        { "first frame OK,             reset session_only",            comp,       ZSTD_reset_session_only, 0 },
        { "first frame bad block,      reset session_only",            bad_block,  ZSTD_reset_session_only, 0 },
        { "first frame bad header,     reset session_only",            bad_header, ZSTD_reset_session_only, 0 },
        { "first frame bad header,     reset session_and_parameters",  bad_header, ZSTD_reset_session_and_parameters, 0 },
        { "first frame bad header,     reset session_only + refPrefix(NULL)", bad_header, ZSTD_reset_session_only, 1 },
    };
    for (size_t i = 0; i < sizeof cases / sizeof cases[0]; i++) {
        if (free_prefix && i != 2) continue;
        ZSTD_DCtx *d = ZSTD_createDCtx();
        ZSTD_DCtx_refPrefix(d, prefix, N);
        size_t ol1, ol, r1 = decode(d, cases[i].first, n, &ol1);
        ZSTD_DCtx_reset(d, cases[i].reset);
        if (cases[i].ref_null) ZSTD_DCtx_refPrefix(d, NULL, 0);
        if (free_prefix) { free(prefix); prefix = NULL; }   /* the failed frame is over; release its prefix */
        size_t r2 = decode(d, comp, n, &ol);                 /* next frame, no prefix referenced for it */
        printf("%-62s first: %-28s then frame P: %s\n", cases[i].name, res(r1, ol1), res(r2, ol));
        ZSTD_freeDCtx(d);
    }
    free(prefix);
    return 0;
}

Expected behavior

One consistent rule that the documentation states. Either of these would remove the surprise:

  1. A session reset drops a prefix that was referenced for the abandoned frame, whatever stage the frame reached (matching "used once" and the reset-after-error guidance); or
  2. The documentation says that the prefix is consumed only when a frame header is accepted, so after an error the caller must call ZSTD_DCtx_refPrefix(dctx, NULL, 0) or reset parameters before releasing the buffer or decoding an unrelated frame.

My reading of the current text is that (1) is intended, but I may be missing a reason for the current behavior.

Desktop (please complete the following information):

  • OS: Linux x86_64
  • Version: dev 49cf517, also v1.5.7
  • Compiler: clang 21.1.8
  • Flags: -O1 -g -fsanitize=address,undefined
  • Build system: Makefile (make -C lib libzstd.a)

Additional context

The program uses ZSTD_decompressStream() with the whole frame available in one input buffer. I did not check other decompression entry points. ZSTD_STATIC_LINKING_ONLY is defined only for ZSTD_frameHeaderSize(), which is used to build the corrupted test input.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions