`lyb_print_value()`: FIXED_BITS branch discards the plugin's length and skips its NULL check

[tosanjay]

Hello,

While auditing the Libyang lib (as a part of my research tool evaluation) I found the following two issues in printer_lyb.c file. Rather than opening multiple issues, i am reporting them via single issue thread. Thanks

Tested on 5.8.6 (master @ 47351e5). Both are still present on devel @ 7658afb ("VERSION bump to version 6.1.11", 2026-08-19), i.e. after the 6.1.10 LYB parser fixes — lyb_print_value() is same between master and devel.

Summary-- The shared root cause

lyb_print_value() has two branches. The variable-size branch asks the plugin for the real length and checks the returned pointer; the FIXED_BITS branch does neither.

/* printer_lyb.c:799 */ val = (void *)print(ctx, value, LY_VALUE_LYB, NULL, &dynamic, NULL);  /* length out-param = NULL */
/* :800 */              LY_CHECK_GOTO(ret, cleanup);       /* dead — see below */
/* :803 */              val_size_bits = fixed_size_bits;   /* schema constant substituted for the real length */
                        /* ... the sibling variable-size branch instead does: */
/* :809 */              LY_CHECK_ERR_GOTO(!val, ret = LY_EINT, cleanup);
/* :823 */              if (val_size_bits > 0) ret = lyb_write(val, val_size_bits, lybctx);   /* val may be NULL */

Two consequences follow:

1. The plugin's value_size_bits out-param is passed as NULL, so whatever length the plugin actually produced is thrown away and the schema constant is used instead.
2. There is no !val check. The LY_CHECK_GOTO(ret, cleanup) at :800 cannot catch a NULL return — ret is LY_SUCCESS from :778 and the print callback returns const void *, so it never assigns ret. The check is dead code.

Finding 1 — a FIXED_BITS type can return variable-length data, and it is silently truncated

lyplg_type_lyb_size_time_nz() declares LYPLG_LYB_SIZE_FIXED_BITS with fixed_size_bits = 32, unconditionally. But lyplg_type_print_time() takes a variable-size path whenever fractions_s is set — it returns 4 + 1 + strlen(fractions) bytes and reports 32 + 8 + strlen*8 bits. time-no-zone in ietf-yang-types explicitly admits fractional seconds (pattern …(\.[0-9]+)?).

Because the caller discards the reported length and writes exactly 32 bits, the fraction and the flag byte are dropped — and lyd_print_all() returns LY_SUCCESS. A valid value is silently destroyed by a print/parse round-trip:

stored value       : 12:34:56.789
print returned 0                       (LY_SUCCESS)
round-tripped value: 12:34:56          *** VALUE CHANGED ACROSS LYB ROUND-TRIP ***

No allocation failure, no injection, no sanitizer needed — this is reproducible with a plain build (harness/time_nz_poc.c, log asan/finding1_truncation_roundtrip.log).

Impact. The loss is confined to the value: sibling nodes printed before and after it survive intact, because the printer writes and the parser reads the same 32 bits, so the stream stays in sync.

orig:    before=SENTINEL-BEFORE  tnz=12:34:56.789  after=SENTINEL-AFTER  num=4242
parsed:  before=SENTINEL-BEFORE  tnz=12:34:56      after=SENTINEL-AFTER  num=4242

But if the type is a list key, the whole blob becomes unreadable. Two entries whose keys differ only in the fraction truncate to the same key, and libyang then refuses its own output:

orig:   [ts=12:34:56.111 data=FIRST] [ts=12:34:56.222 data=SECOND]   entries=2
print returned 0
parse returned 7 (LY_EVALID)   Duplicate instance of "ev". (data path: /t:ev[ts='12:34:56'])

So the printer emits, with LY_SUCCESS, a blob that cannot be parsed back — losing the entire tree, not just the fraction. (harness/keys_collide.c)

All 14 in-tree FIXED_BITS records were audited to see whether this is isolated; time_nz is the only mismatch, and it truncates, which is memory-safe. The audit scripts are included (harness/plugin_pairs.py recovers each lyplg_type_record's (lyb_size, print) pairing — 44 records, 14 FIXED_BITS; harness/fixedbits_nullscan.py decides per callback whether its LY_VALUE_LYB path can return NULL). Worth noting the contract is unenforced in both directions: a FIXED_BITS type whose constant were larger than the plugin's buffer would be an over-read rather than a truncation.

Finding 2 — a NULL plugin return reaches lyb_write() under memory pressure

The same unchecked branch is reached by base types decimal64 and enumeration (big-endian) and by time-no-zone (any architecture), whose LYB-print callbacks allocate and return NULL on failure. decimal64/enum do num = htole64(v); if (num == v) return &v; else { calloc(...); LY_CHECK_RET(!buf, NULL); } — on big-endian the else is the normal path — and time-no-zone does malloc(4 + 1 + strlen(fractions)). A NULL return then reaches lyb_write(NULL, 32, …).

This requires an allocation failure, so it is a robustness defect rather than something an attacker can force.

x86-64  time-no-zone "12:34:56.789", fail malloc(8) #20  -> SIGSEGV 139
        bt: ly_write_(buf=0x0,len=4) <- lyb_write(buf=0x0,count_bits=32) printer_lyb.c:338
            <- lyb_print_value:823 <- lyb_print_node_leaf:1225
s390x   decimal64 "12.34"  control EXIT=0 ; fail calloc(1,8) #6 -> SIGSEGV 139
        enum      "beta"   control EXIT=0 ; fail calloc(1,4) #0 -> SIGSEGV 139

The identical binary and input with no injection exit 0.

Reproducing

Unzip the attached artifacts.zip file.

Dependency: libpcre2-dev, plus cmake and a C compiler. ENABLE_TESTS/ENABLE_VALGRIND_TESTS default to ON for Debug, so pass them off unless you want the CMocka suite built. Finding 2 needs a non-sanitised build: ASan's allocator intercepts calloc/malloc ahead of an LD_PRELOAD interposer, so the fault injector silently never fires under ASan.

# finding 1 — a plain build is enough
cmake -S <libyang> -B build-plain -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_FLAGS="-g -O0" \
  -DENABLE_TESTS=OFF -DENABLE_VALGRIND_TESTS=OFF
cmake --build build-plain -j$(nproc)

Include paths must point into the build tree — libyang.h and ly_config.h are generated at configure time, so -I <libyang> fails on ly_config.h.

gcc -g -O1 -I build-plain/libyang -I build-plain/compat \
    harness/time_nz_poc.c -o time_nz_poc -L build-plain -lyang -Wl,-rpath,build-plain
gcc -shared -fPIC -O1 harness/failmalloc.c -o failmalloc.so -ldl     # finding 2 only
export LYB_MODULES_DIR=<libyang>/modules

./time_nz_poc "12:34:56.789"                                        # finding 1

# finding 2 — enumerate candidate allocations first, then fail the one during the print
LD_PRELOAD=./failmalloc.so FAIL_MALLOC_SIZE=8 FAIL_MALLOC_LOG=1   ./time_nz_poc "12:34:56.789"
LD_PRELOAD=./failmalloc.so FAIL_MALLOC_SIZE=8 FAIL_MALLOC_INDEX=20 ./time_nz_poc "12:34:56.789"

The big-endian results were produced under qemu-s390x (binfmt) in an s390x/ubuntu container via harness/build_and_run.sh. Note qemu user-mode does not implement ptrace, so no gdb backtrace is obtainable there — the x86-64 backtrace pins the same sink.

Suggested fix — patch attached

Two patches against src/printer_lyb.c on master @ 47351e5, deliberately separate so you can take the first without the second — they fix different findings and only the second changes behaviour.

patch/0001-printer-lyb-check-plugin-return-value.patch — one line, fixes finding 2 (the crash):

-        LY_CHECK_GOTO(ret, cleanup);
+        LY_CHECK_ERR_GOTO(!val, ret = LY_EINT, cleanup);

Verified standalone: builds clean, regression suite 0 failures, the injected allocation failure goes from SIGSEGV exit 139 to a clean LY_EINT, and finding 1 is left exactly as it was. No behaviour change for any value that previously printed successfully.

patch/0002-printer-lyb-honour-declared-fixed-size.patch — applies on top, fixes finding 1 (the truncation) by asking the plugin for the length it actually produced and refusing to write a value that disagrees with the declared fixed size:

-        /* get value from plugin */
-        val = (void *)print(ctx, value, LY_VALUE_LYB, NULL, &dynamic, NULL);
-        LY_CHECK_GOTO(ret, cleanup);
+        uint64_t plugin_size_bits = 0;
+
+        /* get value from plugin, including the length it actually produced */
+        val = (void *)print(ctx, value, LY_VALUE_LYB, NULL, &dynamic, &plugin_size_bits);
+        LY_CHECK_ERR_GOTO(!val, ret = LY_EINT, cleanup);
+
+        if (plugin_size_bits != fixed_size_bits) {
+            LOGERR(lybctx->ctx, LY_EINT, "LYB type plugin for \"%s\" declared a fixed size of %" PRIu64
+                    " bits but produced %" PRIu64 " bits.", value->realtype->name, fixed_size_bits, plugin_size_bits);
+            ret = LY_EINT;
+            goto cleanup;
+        }

Verified — full transcript in patch/PATCH_VERIFICATION.log:

check	0001 only	0001 + 0002
builds clean, no new warnings	yes	yes
regression — one leaf of every FIXED_BITS-backed type still prints (boolean, empty, enumeration, decimal64, bits, ipv4/ipv6-address-no-zone, ipv4/ipv6-prefix, time-no-zone without fractions, date-no-zone)	0 failures	0 failures
finding 2 — was SIGSEGV exit 139	fixed — LY_EINT, no crash	fixed
finding 1 — was print returned 0, value silently 12:34:56	unchanged (still truncates)	fixed — declared a fixed size of 32 bits but produced 64 bits, LY_EINT

patch/regress_fixedbits_types.c is the regression harness; it prints one leaf of each affected type and reports any that fail. I checked the case most likely to break: empty is served by the generic lyplg_type_print_simple(), which reports ly_strlen(value->_canonical) * 8 regardless of format — that yields 0 for empty, matching its declared 0, so the equality check is satisfied. Worth knowing that agreement is incidental rather than by construction.

The patch introduces a behaviour change — please weigh it. For a time-no-zone carrying fractional seconds:

	before	after
lyd_print_all() returns	LY_SUCCESS	LY_EINT
output	produced, with the fraction dropped	none
value survives a round-trip	no (12:34:56.789 → 12:34:56)	n/a

So a call that previously reported success now fails. That regression is caused by the patch, not merely exposed by it — though what it replaces is silent data destruction, not working behaviour: there is no correct LYB encoding for such a value today either way. Nothing that was previously correct becomes incorrect.

We think surfacing the loss is the right default, but you may reasonably disagree — erroring on a valid YANG value is arguably worse than a lossy encoding, in which case the fix below is the one you want instead.

That question is yours to decide, and the patch deliberately does not pre-empt it: lyb_size receives only the lysc_type, not the value, so it cannot know whether a particular value carries fractions. If time-no-zone is to support them in LYB at all, lyplg_type_lyb_size_time_nz() has to declare LYPLG_LYB_SIZE_VARIABLE_* for the type as a whole — which changes the wire encoding for that type. We have not patched that, since it is a format decision. Note the current encoding is already lossy for those values, so no correct encoding exists today to preserve.

Bundle

file	finding	role
harness/time_nz_poc.c	1, 2	stores a time-no-zone value with fractions, prints, re-parses
harness/keys_collide.c	1	two list entries keyed on time-no-zone, differing only in the fraction — shows the round-trip failing with LY_EVALID
harness/be_poc.c	2	same shape for decimal64 / enumeration, prints the branch selector before crashing
harness/build_and_run.sh	2	builds libyang + be_poc inside an s390x/ubuntu container
harness/failmalloc.c	2	LD_PRELOAD interposer failing the Nth malloc(SIZE)
harness/failcalloc.c	2	same for calloc(1, SIZE)
harness/plugin_pairs.py	1	recovers each lyplg_type_record's (lyb_size, print) pairing from source
harness/fixedbits_nullscan.py	1, 2	per FIXED_BITS print callback, decides whether its LY_VALUE_LYB path can return NULL
asan/finding1_truncation_roundtrip.log	1	12:34:56.789 → 12:34:56, print returns LY_SUCCESS
asan/finding2_nullderef_confirmed.log	2	the injected run: FAILING malloc(8) #20 → SIGSEGV, exit 139
asan/finding2_backtrace.log	2	gdb backtrace pinning the sink at printer_lyb.c:823
asan/finding2_s390x_bigendian.log	2	controls (exit 0) and both injected crashes (exit 139)
asan/s390x_build.log	2	build provenance for the emulated s390x libyang
patch/0001-printer-lyb-check-plugin-return-value.patch	2	one-line NULL check; no behaviour change
patch/0002-printer-lyb-honour-declared-fixed-size.patch	1	applies on top of 0001; introduces the behaviour change below
patch/regress_fixedbits_types.c	1, 2	regression harness — prints one leaf of every FIXED_BITS-backed type
patch/PATCH_VERIFICATION.log	1, 2	build + regression + before/after transcript for the patch

The two .py files are analysis aids, not exploits — run them with python3 harness/<script>.py against a libyang source tree (edit the SRC path at the top of each). Logs are verbatim and unedited.

artifacts.zip