archive_acl: Fix buffer overrun and wrong output for NULL-name ACL entries

archive_acl_text_len() counted the trailing ":id" digits only when
ARCHIVE_ENTRY_ACL_STYLE_EXTRA_ID was set, but archive_acl_to_text_l()
always writes them for USER/GROUP entries whose name is NULL.  With a
7-digit or larger id the allocated buffer was too short, causing
append_id() to write past its end.

Fix the estimator to also count the extra colon and digits when the
name is NULL, matching the serializer's logic.

The wide serializer (archive_acl_to_text_w) had the opposite problem:
it passed id=-1 to append_entry_w() for NULL-name entries regardless
of the id value, causing a garbage character to be written in the name
field and the numeric id to be omitted entirely.  Fix it to mirror the
narrow serializer by setting id = ap->id when wname is NULL.

Add tests for both the narrow and wide paths.

diff --git a/Makefile.am b/Makefile.am
index 7417b00c90df2f9ad87336de9b13f6142d9e9001..b19e6837cb8da71a91705f797fc57262d09e3341 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -377,6 +377,7 @@ libarchive_test_SOURCES= \
        libarchive/test/read_open_memory.c \
        libarchive/test/test_7zip_filename_encoding.c \
        libarchive/test/test_acl_nfs4.c \
+       libarchive/test/test_acl_nfs4_null_id_overflow.c \
        libarchive/test/test_acl_pax.c \
        libarchive/test/test_acl_platform_nfs4.c \
        libarchive/test/test_acl_platform_posix1e.c \

diff --git a/libarchive/archive_acl.c b/libarchive/archive_acl.c
index 14a107a47ccd5e2a6e6b0d9f213b3de7bb436f5f..af332eb20a48330c9a0e19057d10025123a70453 100644 (file)
--- a/libarchive/archive_acl.c
+++ b/libarchive/archive_acl.c
@@ -638,7 +638,8 @@ archive_acl_text_len(struct archive_acl *acl, int want_type, int flags,
 
                if ((ap->tag == ARCHIVE_ENTRY_ACL_USER ||
                    ap->tag == ARCHIVE_ENTRY_ACL_GROUP) &&
-                   (flags & ARCHIVE_ENTRY_ACL_STYLE_EXTRA_ID) != 0) {
+                   ((flags & ARCHIVE_ENTRY_ACL_STYLE_EXTRA_ID) != 0 ||
+                   (wide ? wname == NULL : name == NULL))) {
                        length += 1; /* colon */
                        /* ID digit count */
                        idlen = 1;
@@ -750,7 +751,8 @@ archive_acl_to_text_w(struct archive_acl *acl, ssize_t *text_len, int flags,
                if (r == 0) {
                        if (count > 0)
                                *wp++ = separator;
-                       if (flags & ARCHIVE_ENTRY_ACL_STYLE_EXTRA_ID)
+                       if ((flags & ARCHIVE_ENTRY_ACL_STYLE_EXTRA_ID) ||
+                           wname == NULL)
                                id = ap->id;
                        else
                                id = -1;

diff --git a/libarchive/test/CMakeLists.txt b/libarchive/test/CMakeLists.txt
index d574fa5a8feefc8f1c36e49d3413dd3da5206b97..2d2ff013f9cae6d0f0a4092785e7d5305cc465d9 100644 (file)
--- a/libarchive/test/CMakeLists.txt
+++ b/libarchive/test/CMakeLists.txt
@@ -11,6 +11,7 @@ IF(ENABLE_TEST)
     test.h
     test_7zip_filename_encoding.c
     test_acl_nfs4.c
+    test_acl_nfs4_null_id_overflow.c
     test_acl_pax.c
     test_acl_platform_nfs4.c
     test_acl_platform_posix1e.c

diff --git a/libarchive/test/test_acl_nfs4_null_id_overflow.c b/libarchive/test/test_acl_nfs4_null_id_overflow.c
new file mode 100644 (file)
index 0000000..590c704
--- /dev/null
+++ b/libarchive/test/test_acl_nfs4_null_id_overflow.c
@@ -0,0 +1,105 @@
+/*-
+ * Copyright (c) 2026 Tim Kientzle
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR(S) ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR(S) BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#include "test.h"
+
+/*
+ * A USER NFSv4 ACL entry with a NULL name and a large numeric qualifier
+ * triggers a heap buffer overflow in archive_acl_to_text_l().
+ *
+ * archive_acl_text_len() only reserves space for the trailing ":id" digits
+ * when ARCHIVE_ENTRY_ACL_STYLE_EXTRA_ID is set, but the serializer always
+ * writes them for USER/GROUP entries when the name is NULL.  With a 7-digit
+ * ID the allocated buffer is too short and append_id() writes past its end.
+ *
+ * Without the fix, libarchive's own post-write guard fires:
+ *   "Fatal Internal Error: Buffer overrun"
+ * (or under AddressSanitizer a heap-buffer-overflow is reported at
+ * archive_acl.c in append_id / append_entry).
+ */
+DEFINE_TEST(test_acl_nfs4_null_id_overflow)
+{
+       struct archive_entry *ae;
+       char *text;
+       ssize_t len;
+
+       ae = archive_entry_new();
+       assert(ae != NULL);
+       archive_entry_set_pathname(ae, "file");
+       archive_entry_set_mode(ae, AE_IFREG | 0644);
+
+       /* NULL name + 7-digit id: the missing ":9999999" overflows the buffer */
+       archive_entry_acl_add_entry(ae,
+           ARCHIVE_ENTRY_ACL_TYPE_DENY,
+           0,
+           ARCHIVE_ENTRY_ACL_USER,
+           9999999,
+           NULL);
+
+       text = archive_entry_acl_to_text(ae, &len,
+           ARCHIVE_ENTRY_ACL_TYPE_NFS4);
+       assert(text != NULL);
+       /* The numeric id must appear in the output as the user name */
+       assert(strstr(text, "9999999") != NULL);
+       free(text);
+
+       archive_entry_free(ae);
+}
+
+/*
+ * The wide (wchar_t) serializer has a related bug: when the name is NULL
+ * it passes id=-1 to append_entry_w() instead of the actual numeric id,
+ * so the id is never written and a garbage character appears in the name
+ * slot instead.
+ *
+ * The wide estimator has the same missing-id-digits gap as the narrow one,
+ * and both must be fixed together with the wide serializer.
+ */
+DEFINE_TEST(test_acl_nfs4_null_id_overflow_w)
+{
+       struct archive_entry *ae;
+       wchar_t *wtext;
+       ssize_t len;
+
+       ae = archive_entry_new();
+       assert(ae != NULL);
+       archive_entry_set_pathname(ae, "file");
+       archive_entry_set_mode(ae, AE_IFREG | 0644);
+
+       archive_entry_acl_add_entry(ae,
+           ARCHIVE_ENTRY_ACL_TYPE_DENY,
+           0,
+           ARCHIVE_ENTRY_ACL_USER,
+           9999999,
+           NULL);
+
+       wtext = archive_entry_acl_to_text_w(ae, &len,
+           ARCHIVE_ENTRY_ACL_TYPE_NFS4);
+       assert(wtext != NULL);
+       /* The numeric id must appear in the output as the user name */
+       assert(wcsstr(wtext, L"9999999") != NULL);
+       free(wtext);
+
+       archive_entry_free(ae);
+}