Control: tag -1 patch Hi!
This was my reply with analysis and queued fix. ----- Forwarded message from Guillem Jover <[email protected]> ----- Date: Tue, 28 Jul 2026 14:34:37 +0200 From: Guillem Jover <[email protected]> To: zer0d4y5 <[email protected]> Cc: [email protected] Subject: Re: NULL pointer dereference in dpkg control/Packages parser (empty first field) Message-ID: <[email protected]> Hi! [ Sorry, only saw this yesterday as it was in my spam box. :/ ] I'm CCing the Debian Security Team, which get involved in security affairs related to dpkg. Mail only very minimally trimmed, to give context to them. On Sun, 2026-07-26 at 20:42:27 -0400, zer0d4y5 wrote: > I found a small but reproducible crash in dpkg's control-stanza parser and > wanted to > report it privately first so it can be fixed before any public writeup. > > Parsing a control or Packages stanza whose first field has an empty value, > when that > field is handled by a value-dereferencing parser (Version is the clean > case), crashes > dpkg with a NULL pointer dereference. It is a denial of service, not memory > corruption > or code execution, but it does take down dpkg on malformed input where a > clean error > would be expected. Thanks for finding and reporting this! Given that this also affects «dpkg-deb -f foo.deb Field» which then causes temporary files to linger around (although usually very small ones), and the interfaces is considered (in its man page) a security boundary, I'm inclined to consider this a DoS security issue (but probably low impact, affecting automated systems that might be processing untrusted binaries). I assume commit ae03bfd28afea2fdc10be9657d78a1fe29dfa58a introduced this regression, but need to verify, which might mean this is present since dpkg 1.22.7, affecting Debian trixi and later. I've tested on chroots, and this indeed does not affect Debian bookworm, and affects Debian trixie. I'm not sure whether the Debian Security Team, would consider this enough for a CVE and a security upload though. > Root cause: in lib/dpkg/parse.c, pkg_parse_field() passes fs->value.buf to > the field > handler. For an empty value, varbuf_set_buf() calls varbuf_add_buf(..., 0), > which returns > early without allocating, so when this is the first field parsed in a > stanza the varbuf > has never grown and fs->value.buf is still NULL (fs is memset to 0 at the > top of > parsedb_parse). f_version() then reaches parse_db_version() and > parseversion(), which > dereferences it at parsehelp.c:250 (while (*string ...)) with no NULL guard. Right. The underlying problem though is the varbuf rework which was in theory done to make the API usage safer, but has already caused some other segfaults and crashes along the way. I'm not happy yet with the current API, but was not really happy with the previous state of it either. :/ > Trigger (10 bytes): > > printf 'Version:\nz\n' > crash.pkgs > > Reproduced on the shipped Ubuntu 26.04 dpkg 1.23.7 and on current git HEAD > (1.23.7-203-g7004a048f), both segfault (exit 139, core dumped): > > dpkg --merge-avail crash.pkgs # also --update-avail > dpkg --install <a .deb whose control's first field is an empty Version:> > > apt is not affected (it has its own parser), and dpkg-deb --info/--field > only dump the > control text without semantic parsing, so they do not crash. The reachable > consumers are > libdpkg's parsedb callers. > > Suggested fix (one line, verified): pass a guaranteed C string to the field > handlers, for > example varbuf_str(&fs->value) instead of fs->value.buf in > pkg_parse_field(). varbuf_str() > returns "" for a NULL buffer, so parseversion cleanly reports "version > string is empty". I > rebuilt current git HEAD with this change and the same input now returns > > dpkg: error: parsing file 'crash.pkgs' near line 1: > 'Version' field value '': version string is empty > > with exit 2, instead of crashing. This one change covers every field > handler, not just > Version. While doing another quick check over the whole code base I also noticed a problem with dependency fields, with empty version strings, such as: # dpkg --update-avail <<CTRL Package: test Version: 1.0 Depends: pkgname (= ) Maintainer: Maint <[email protected]> Description: test package CTRL Which also segfaults, but would not be a security issue. So instead I've prepared the attached change, which should be more complete. But I'd like to give another review pass over the codebase to make sure, and check that the new semantic changes do not break expectations of existing code assuming a NULL for zero length strings. > I am happy to share the exact .deb reproducer and a patch in whatever form > is easiest. No worries, the description was enough to generate reproducers for this. > No rush on my end, and I will hold off on any public description until you > have had a > chance to look and a fix is available. Please let me know a good way to > coordinate. Thank you! Let's see what the Security Team have to say, and then we can decide on how to proceed, or whether a schedule is needed, etc. Also let me know whether the current attribution is fine or you'd want some other name or email address being used, to use once I write the commit message, etc. Regards, Guillem Reported-by: zer0d4y5 <[email protected]> diff --git i/lib/dpkg/t/t-varbuf-cpp.cc w/lib/dpkg/t/t-varbuf-cpp.cc index 14d127b37..378344a8f 100644 --- i/lib/dpkg/t/t-varbuf-cpp.cc +++ w/lib/dpkg/t/t-varbuf-cpp.cc @@ -509,8 +509,8 @@ test_varbuf_detach(void) vb.init(0); vb.add_buf(nullptr, 0); test_pass(vb.used == 0); - test_pass(vb.size == 0); - test_pass(vb.buf == nullptr); + test_pass(vb.size > 0); + test_pass(vb.buf != nullptr); str = vb.detach(); test_str(str, ==, ""); test_pass(vb.used == 0); diff --git i/lib/dpkg/t/t-varbuf.c w/lib/dpkg/t/t-varbuf.c index 67af373d9..a1b23e16f 100644 --- i/lib/dpkg/t/t-varbuf.c +++ w/lib/dpkg/t/t-varbuf.c @@ -581,8 +581,8 @@ test_varbuf_detach(void) varbuf_init(&vb, 0); varbuf_add_buf(&vb, NULL, 0); test_pass(vb.used == 0); - test_pass(vb.size == 0); - test_pass(vb.buf == NULL); + test_pass(vb.size > 0); + test_pass(vb.buf != NULL); str = varbuf_detach(&vb); test_str(str, ==, ""); test_pass(vb.used == 0); diff --git i/lib/dpkg/varbuf.c w/lib/dpkg/varbuf.c index 2a4434e94..6626d6d5f 100644 --- i/lib/dpkg/varbuf.c +++ w/lib/dpkg/varbuf.c @@ -178,12 +178,11 @@ varbuf_add_char(struct varbuf *v, int c) void varbuf_dup_char(struct varbuf *v, int c, size_t n) { - if (n == 0) - return; - varbuf_grow(v, n + 1); - memset(v->buf + v->used, c, n); - v->used += n; + if (n > 0) { + memset(v->buf + v->used, c, n); + v->used += n; + } v->buf[v->used] = '\0'; } @@ -208,11 +207,11 @@ varbuf_add_dir(struct varbuf *v, const char *dirname) void varbuf_add_buf(struct varbuf *v, const void *s, size_t size) { - if (size == 0) - return; varbuf_grow(v, size + 1); - memcpy(v->buf + v->used, s, size); - v->used += size; + if (size > 0) { + memcpy(v->buf + v->used, s, size); + v->used += size; + } v->buf[v->used] = '\0'; } ----- End forwarded message ----- Thanks, Guillem
