From: Dmitry Ivankov <dmitry.ivankov@cognite.com>
Date: Tue, 22 Apr 2025 14:29:44 +0000 (+0200)
Subject: bsdtar: don't hardlink negative inode files together (#2587)
X-Git-Tag: v3.8.0~39
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=be8d08868c9bd9ebc2e27eeb7c3df74e4749bb91;p=thirdparty%2Flibarchive.git

bsdtar: don't hardlink negative inode files together (#2587)

It seems that valid inode can be 64-bit and negative (or rather outside
of 64-bit signed range)


https://github.com/freebsd/freebsd-src/blob/a60615d5be83ca050d4ddfbbb4411ca7a8a11486/sys/sys/_types.h#L124
https://github.com/torvalds/linux/blob/7e74f756f5f643148ca5537bf2fee6767e4b0ed9/include/linux/types.h#L22

But signed type is used in libarchive and there were some fuzzing issues
with it, https://github.com/libarchive/libarchive/pull/2258 converts
negative `ino` to `0`, which is actually a reserved inode value, but
more importantly it was still setting `AE_SET_INO` flag and later on
hardlink detection will treat all `0` on same `dev` as hardlinks to each
other if they have some hardlinks.

This showed up in BuildBarn FUSE filesystem
https://github.com/buildbarn/bb-remote-execution/issues/162 which has
both
- setting number of links to a big value
- generating random inode values in full uint64 range

Which in turn triggers false-positive hardlink detection in `bsdtar`
with high probability.

Let's mitigate it
- don't set `AE_SET_INO` on negative values (assuming rest of code isn't
ready to correctly handle full uint64 range)
- check that `ino + dev` are set in link resolver
---

diff --git a/libarchive/archive_entry.c b/libarchive/archive_entry.c
index f20bb2efc..c727b9cd8 100644
--- a/libarchive/archive_entry.c
+++ b/libarchive/archive_entry.c
@@ -984,7 +984,9 @@ void
 archive_entry_set_ino(struct archive_entry *entry, la_int64_t ino)
 {
 	if (ino < 0) {
-		ino = 0;
+		entry->stat_valid = 0;
+		entry->ae_set &= ~AE_SET_INO;
+		return;
 	}
 	entry->stat_valid = 0;
 	entry->ae_set |= AE_SET_INO;
@@ -995,7 +997,9 @@ void
 archive_entry_set_ino64(struct archive_entry *entry, la_int64_t ino)
 {
 	if (ino < 0) {
-		ino = 0;
+		entry->stat_valid = 0;
+		entry->ae_set &= ~AE_SET_INO;
+		return;
 	}
 	entry->stat_valid = 0;
 	entry->ae_set |= AE_SET_INO;
diff --git a/libarchive/archive_entry_link_resolver.c b/libarchive/archive_entry_link_resolver.c
index c2fd6895f..77fcad61f 100644
--- a/libarchive/archive_entry_link_resolver.c
+++ b/libarchive/archive_entry_link_resolver.c
@@ -280,6 +280,10 @@ find_entry(struct archive_entry_linkresolver *res,
 	dev_t			 dev;
 	int64_t			 ino;
 
+	if (!archive_entry_ino_is_set(entry) || !archive_entry_dev_is_set(entry)) {
+		return (NULL);
+	}
+
 	/* Free a held entry. */
 	if (res->spare != NULL) {
 		archive_entry_free(res->spare->canonical);
@@ -369,6 +373,10 @@ insert_entry(struct archive_entry_linkresolver *res,
 	struct links_entry *le;
 	size_t hash, bucket;
 
+	if (!archive_entry_ino_is_set(entry) || !archive_entry_dev_is_set(entry)) {
+		return (NULL);
+	}
+
 	/* Add this entry to the links cache. */
 	le = calloc(1, sizeof(struct links_entry));
 	if (le == NULL)