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 f20bb2efc7002f012741015ef0a7dbce678986cf..c727b9cd85f34d0bacd7ef3079660521e0225465 100644 (file)
--- 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 c2fd6895f21e97b7b70ea6ed9c3384f0ccf4a4dc..77fcad61fdeaf5748ae1be77b676a7724ae86fc4 100644 (file)
--- 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)