From: Martin Matuska <martin@matuska.org>
Date: Fri, 31 Jan 2020 23:58:03 +0000 (+0100)
Subject: Fix use after free in ISO9660 and XAR writer
X-Git-Tag: v3.4.2~15
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8705a85c7e74ee50227ed57fd88a390aa063f619;p=thirdparty%2Flibarchive.git

Fix use after free in ISO9660 and XAR writer

Remove rbtree nodes before freeing them.
For better code readability import new rbtree macros from NetBSD.

Fixes #1325
---

diff --git a/libarchive/archive_rb.h b/libarchive/archive_rb.h
index c6d11468d..8851f1081 100644
--- a/libarchive/archive_rb.h
+++ b/libarchive/archive_rb.h
@@ -49,12 +49,24 @@ struct archive_rb_node {
     __archive_rb_tree_iterate((T), NULL, ARCHIVE_RB_DIR_LEFT)
 #define ARCHIVE_RB_TREE_MAX(T) \
     __archive_rb_tree_iterate((T), NULL, ARCHIVE_RB_DIR_RIGHT)
+#define ARCHIVE_RB_TREE_NEXT(T, N) \
+    __archive_rb_tree_iterate((T), (N), ARCHIVE_RB_DIR_RIGHT)
+#define ARCHIVE_RB_TREE_PREV(T, N) \
+    __archive_rb_tree_iterate((T), (N), ARCHIVE_RB_DIR_LEFT)
 #define ARCHIVE_RB_TREE_FOREACH(N, T) \
     for ((N) = ARCHIVE_RB_TREE_MIN(T); (N); \
-	(N) = __archive_rb_tree_iterate((T), (N), ARCHIVE_RB_DIR_RIGHT))
+	(N) = ARCHIVE_RB_TREE_NEXT((T), (N)))
 #define ARCHIVE_RB_TREE_FOREACH_REVERSE(N, T) \
     for ((N) = ARCHIVE_RB_TREE_MAX(T); (N); \
-	(N) = __archive_rb_tree_iterate((T), (N), ARCHIVE_RB_DIR_LEFT))
+	(N) = ARCHIVE_RB_TREE_PREV((T), (N)))
+#define ARCHIVE_RB_TREE_FOREACH_SAFE(N, T, S) \
+    for ((N) = ARCHIVE_RB_TREE_MIN(T); \
+	(N) && ((S) = ARCHIVE_RB_TREE_NEXT((T), (N)), 1); \
+	(N) = (S))
+#define ARCHIVE_RB_TREE_FOREACH_REVERSE_SAFE(N, T, S) \
+    for ((N) = ARCHIVE_RB_TREE_MAX(T); \
+        (N) && ((S) = ARCHIVE_RB_TREE_PREV((T), (N)), 1); \
+        (N) = (S))
 
 /*
  * archive_rbto_compare_nodes_fn:
diff --git a/libarchive/archive_write_set_format_iso9660.c b/libarchive/archive_write_set_format_iso9660.c
index e3020cc92..7cde44c34 100644
--- a/libarchive/archive_write_set_format_iso9660.c
+++ b/libarchive/archive_write_set_format_iso9660.c
@@ -5094,13 +5094,11 @@ isofile_init_hardlinks(struct iso9660 *iso9660)
 static void
 isofile_free_hardlinks(struct iso9660 *iso9660)
 {
-	struct archive_rb_node *n, *next;
+	struct archive_rb_node *n, *tmp;
 
-	for (n = ARCHIVE_RB_TREE_MIN(&(iso9660->hardlink_rbtree)); n;) {
-		next = __archive_rb_tree_iterate(&(iso9660->hardlink_rbtree),
-		    n, ARCHIVE_RB_DIR_RIGHT);
+	ARCHIVE_RB_TREE_FOREACH_SAFE(n, &(iso9660->hardlink_rbtree), tmp) {
+		__archive_rb_tree_remove_node(&(iso9660->hardlink_rbtree), n);
 		free(n);
-		n = next;
 	}
 }
 
diff --git a/libarchive/archive_write_set_format_xar.c b/libarchive/archive_write_set_format_xar.c
index ab0c6b48a..d456cf8f8 100644
--- a/libarchive/archive_write_set_format_xar.c
+++ b/libarchive/archive_write_set_format_xar.c
@@ -2563,13 +2563,11 @@ file_init_hardlinks(struct xar *xar)
 static void
 file_free_hardlinks(struct xar *xar)
 {
-	struct archive_rb_node *n, *next;
+	struct archive_rb_node *n, *tmp;
 
-	for (n = ARCHIVE_RB_TREE_MIN(&(xar->hardlink_rbtree)); n;) {
-		next = __archive_rb_tree_iterate(&(xar->hardlink_rbtree),
-		    n, ARCHIVE_RB_DIR_RIGHT);
+	ARCHIVE_RB_TREE_FOREACH_SAFE(n, &(xar->hardlink_rbtree), tmp) {
+		__archive_rb_tree_remove_node(&(xar->hardlink_rbtree), n);
 		free(n);
-		n = next;
 	}
 }