lyd_parse_data_mem() accepts LYD_CBOR but cannot supply its required buffer length

[karowski]

lyd_parse_data_mem() accepts LYD_CBOR but cannot supply its required buffer length

• Component: libyang, src/tree_data.c (lyd_parse_data_mem), src/lcbor.c
• Affected: upstream devel only (CBOR is devel-only; LYD_CBOR is not in v5.8.6 or any release
  tag). Confirmed on local libyang devel 6.1.8; the relevant code — lyd_parse_data_mem()
  (tree_data.c:240), ly_in_new_memory() (in.c:183), and the cbor_load() call at lcbor.c:102
  — is unchanged through current upstream devel d80d46ea (v5.8.6-147-gd80d46ea) by source
  comparison.
• Severity: Low — usability / diagnostics, not a memory-safety or security issue. Parsing a
  valid CBOR document from memory with lyd_parse_data_mem() fails with an error that blames the
  input, when the real cause is that the API cannot convey a buffer length for CBOR. No
  crash, no out-of-bounds access, no incorrect data accepted — the document is simply rejected with a
  wrong reason. Reachable only in a CBOR-enabled devel build.

What happens

The same valid document {"cbor-test:x": "v"} is parsed two ways:

document: 15 bytes, valid {"cbor-test:x": "v"}

[A] lyd_parse_data_mem()      -> rc=7 (ERROR)  msg: Failed to parse CBOR data.
[B] lyd_parse_data_mem_len()  -> rc=0 (ok)     msg: accepted

[A] returns LY_EVALID with "Failed to parse CBOR data."; [B], given the identical bytes plus
an explicit length, accepts it. So the data is fine — only the _mem entry point fails, and its
message points at the data rather than at the call.

Root cause

lyd_parse_data_mem() (tree_data.c:240) builds its input handle with ly_in_new_memory()
(in.c:183), which takes only (data, in) and therefore never sets in->length — it stays 0. For
CBOR that is fatal: lcbor.c:102 calls cbor_load(in->current, in->length, &result) with
that 0, and libcbor cannot parse a zero-length buffer, so libyang logs "Failed to parse CBOR data."
and returns LY_EVALID.

lyd_parse_data_mem_len() (tree_data.c:223) is the counterpart that sets in->length and
in->bounded explicitly; it parses the same bytes correctly. It was added in the same commit that
introduced CBOR, i.e. it is the intended entry point for binary input from memory.

The defect is therefore not that _mem "should" parse CBOR — for a NUL-terminated buffer it has no
length to give — but that the failure is silent and misdirected:

• lyd_parse_data_mem()'s format parameter accepts LYD_CBOR with no up-front rejection,
• the documentation never states that CBOR needs the length-bearing lyd_parse_data_mem_len(). The
  nearest clue is that function's own note that its data "may contain NULL bytes and the parser
  will not read past data_len" (parser_data.h:272) — which hints at binary input without ever
  saying CBOR requires it — and lyd_parse_data_mem_len() is itself omitted from the parser function
  list (parser_data.h:76-78, which names _data / _mem / _fd only), and
• the error it produces implicates the input data rather than the API misuse.

(For contrast, LYD_LYB — also binary — happens to parse through _mem because LYB is
self-delimiting and does not need the length up front; CBOR is the format where the missing length is
immediately fatal.)

Suggested remedy

A maintainer decision; two reasonable options:

• reject LYD_CBOR in lyd_parse_data_mem() up front with a clear message such as "the LYD_CBOR
  format requires a known input length; use lyd_parse_data_mem_len()", or
• document the restriction on lyd_parse_data_mem() explicitly.

Only LYD_CBOR is affected. LYD_LYB — also binary — parses correctly through lyd_parse_data_mem()
because the LYB format is self-delimiting and does not need the buffer length up front (verified:
a printed LYB document round-trips through lyd_parse_data_mem() with LY_SUCCESS), so it must
not be rejected.

No patch is shipped with this ticket.

Reproducer

ly_cbor_parse_mem_no_length.c — public API only. It parses one valid CBOR document via
lyd_parse_data_mem() (the finding) and via lyd_parse_data_mem_len() (the control, proving the
document is valid), and reports the finding only when _mem errors while _mem_len accepts. It must
be built against devel — LYD_CBOR is not declared in the v5.8.6 headers.

/*
 * Reproducer: lyd_parse_data_mem() rejects a VALID CBOR document with a misleading error.
 *
 * lyd_parse_data_mem() builds its input with ly_in_new_memory(), which never sets in->length
 * (it has no length argument). lcbor.c then calls cbor_load(in->current, in->length=0, ...),
 * which cannot parse a zero-length buffer and fails -- so libyang reports "Failed to parse CBOR
 * data." for a perfectly well-formed document, blaming the input rather than indicating that the
 * wrong entry point was used. lyd_parse_data_mem_len() supplies the length explicitly and parses
 * the same bytes fine.
 *
 * This is NOT a memory-safety issue: there is no crash and no out-of-bounds access (cbor_load on a
 * 0-length buffer just returns "not enough data"). It is a usability / diagnostics defect: the
 * public API accepts the LYD_CBOR format argument but the docs never state that CBOR requires the
 * length-bearing lyd_parse_data_mem_len() (which is omitted from the parser function list), and the
 * resulting error message points at the data instead of the API misuse.
 *
 * CBOR is devel-only (LYD_CBOR is absent from v5.8.6 headers), so this must be built against devel.
 *
 * Structure:
 *   [A] the finding  -- lyd_parse_data_mem(valid CBOR)      expected: error (the defect)
 *   [B] the control  -- lyd_parse_data_mem_len(same bytes)  expected: accepted (proves doc valid)
 *
 */

#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>

#include <libyang/libyang.h>

#ifndef LY_MODULES_DIR_DEFAULT
# define LY_MODULES_DIR_DEFAULT NULL
#endif

static const char *module_text =
        "module cbor-test {\n"
        "  namespace \"urn:cbor-test\";\n"
        "  prefix ct;\n"
        "  leaf x { type string; }\n"
        "}\n";

/*
 * {"cbor-test:x": "v"}  -- a valid document
 *
 * A1                                map(1)
 *   6B "cbor-test:x"                text(11)
 *   61 'v'                          text(1)
 */
static const unsigned char cbor_doc[] = {
    /* 15-byte CBOR document, followed by a NUL sentinel so the buffer is also safe under a
     * NUL-terminated interpretation of lyd_parse_data_mem()'s input -- it fails regardless. */
    0xA1,
    0x6B, 'c', 'b', 'o', 'r', '-', 't', 'e', 's', 't', ':', 'x',
    0x61, 'v',
    0x00  /* sentinel, not part of the CBOR document */
};
#define CBOR_DOC_LEN ((uint32_t)(sizeof cbor_doc - 1))

int
main(void)
{
    const char *mod_dir = getenv("LY_MODULES_DIR");
    struct ly_ctx *ctx = NULL;
    struct lyd_node *tree = NULL;
    LY_ERR rc_mem, rc_len;
    int ret = 2;

    setvbuf(stdout, NULL, _IOLBF, 0);

    if (!mod_dir) {
        mod_dir = LY_MODULES_DIR_DEFAULT;
    }
    if (ly_ctx_new(mod_dir, 0, &ctx)) {
        printf("ly_ctx_new failed (module dir \"%s\")\n", mod_dir ? mod_dir : "(none)");
        return 2;
    }
    if (lys_parse_mem(ctx, module_text, LYS_IN_YANG, NULL)) {
        printf("lys_parse_mem failed: %s\n", ly_last_errmsg());
        goto cleanup;
    }

    printf("document: %u bytes, valid {\"cbor-test:x\": \"v\"}\n\n", CBOR_DOC_LEN);

    /* [A] the finding: no explicit length */
    rc_mem = lyd_parse_data_mem(ctx, (const char *)cbor_doc, LYD_CBOR, 0, LYD_VALIDATE_PRESENT, &tree);
    printf("[A] lyd_parse_data_mem()      -> rc=%d %-8s msg: %s\n", rc_mem,
            rc_mem ? "(ERROR)" : "(ok)", rc_mem ? ly_last_errmsg() : "-");
    lyd_free_all(tree);
    tree = NULL;

    /* [B] the control: same bytes, explicit length */
    rc_len = lyd_parse_data_mem_len(ctx, (const char *)cbor_doc, CBOR_DOC_LEN,
            LYD_CBOR, 0, LYD_VALIDATE_PRESENT, &tree);
    printf("[B] lyd_parse_data_mem_len()  -> rc=%d %-8s msg: %s\n", rc_len,
            rc_len ? "(ERROR)" : "(ok)", rc_len ? ly_last_errmsg() : "accepted");
    lyd_free_all(tree);

    printf("\n");
    if (rc_len) {
        printf("  --> the control could not parse the document, so nothing is attributable\n");
        printf("RESULT: inconclusive\n");
        ret = 2;
    } else if (rc_mem) {
        printf("  --> FINDING: lyd_parse_data_mem() rejected a document that lyd_parse_data_mem_len()\n");
        printf("      accepts. The message blames the data (\"Failed to parse CBOR data\") when the real\n");
        printf("      cause is that _mem cannot convey a buffer length for CBOR.\n");
        printf("RESULT: reproduced\n");
        ret = 1;
    } else {
        printf("  --> ok: lyd_parse_data_mem() accepted the document\n");
        printf("RESULT: not reproduced\n");
        ret = 0;
    }

cleanup:
    ly_ctx_destroy(ctx);
    return ret;
}

[michalvasko]

Okay, should be fixed with a nicer error message.