From cf6d04f9f2fcb7a4f12d35edd0a6d4cd5591b59c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Sun, 11 Jul 2021 10:06:12 +0200
Subject: [PATCH] url-mode: fix crash when removing duplicate and/or
 overlapping URLs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Removing overlaping and duplicated URLs is done by running two nested
loops, that both iterate the same URL list.

When a duplicate is found, one of the URLs is destroyed and removed
from the list.

Deleting and removing an item *is* safe, but only as long as _no
other_ iterator has references to it.

In this case, if we remove an item from e.g. the inner iterator, we’ll
crash if the outer iterator’s *next* item is the item being removed.

Closes #627
---
 CHANGELOG.md |  2 ++
 terminal.h   |  1 +
 url-mode.c   | 18 +++++++++++-------
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 44093dc5..acea5f9c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -51,6 +51,8 @@
   last column, and `tweak.allow-overflowing-double-width-glyphs=yes`.
 * FD exhaustion when repeatedly entering/exiting URL mode with many
   URLs.
+* Double free of URL while removing duplicated and/or overlapping URLs
+  in URL mode (https://codeberg.org/dnkl/foot/issues/627).
 
 
 ### Security
diff --git a/terminal.h b/terminal.h
index 94c1cf75..309f7db6 100644
--- a/terminal.h
+++ b/terminal.h
@@ -264,6 +264,7 @@ struct url {
     enum url_action action;
     bool url_mode_dont_change_url_attr; /* Entering/exiting URL mode doesn’t touch the cells’ attr.url */
     bool osc8;
+    bool duplicate;
 };
 typedef tll(struct url) url_list_t;
 
diff --git a/url-mode.c b/url-mode.c
index 8ac5631e..94c05b3a 100644
--- a/url-mode.c
+++ b/url-mode.c
@@ -470,16 +470,20 @@ remove_overlapping(url_list_t *urls, int cols)
                  */
                 xassert(in->osc8 || out->osc8);
 
-                if (in->osc8) {
-                    url_destroy(&outer->item);
-                    tll_remove(*urls, outer);
-                } else {
-                    url_destroy(&inner->item);
-                    tll_remove(*urls, inner);
-                }
+                if (in->osc8)
+                    outer->item.duplicate = true;
+                else
+                    inner->item.duplicate = true;
             }
         }
     }
+
+    tll_foreach(*urls, it) {
+        if (it->item.duplicate) {
+            url_destroy(&it->item);
+            tll_remove(*urls, it);
+        }
+    }
 }
 
 void