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 c6d11468d628f14c46f518a79890c9400274fe3e..8851f1081867da25dde9dab14782193eb9667e28 100644 (file)
--- 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 e3020cc92ea76ddd5a0eae2d9a47a07df2d50a40..7cde44c34f752ca68545a2e8ade05a8c119f7ee1 100644 (file)
--- 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 ab0c6b48abe0d865dee235b105bbee9230af9562..d456cf8f8aa975d075cd3a39a4145c1ff9912a04 100644 (file)
--- 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;
        }
 }