From: Michael Orlitzky <michael@orlitzky.com>
Date: Tue, 22 Oct 2024 11:39:50 +0000 (-0400)
Subject: src/svgtiny.c: improve select_ctx cleanup and error handling
X-Git-Url: https://gitweb.michael.orlitzky.com/?a=commitdiff_plain;h=6645b3e57f22801d8725fa5d5c3f003465b1e4e5;p=libsvgtiny.git

src/svgtiny.c: improve select_ctx cleanup and error handling

In the "cleanup:" routine of svgtiny_parse(), we make a few
improvements:

  * The sheets are now removed from the context before destroying
    them. This avoids a potential issue where destroying the context
    itself (which we do afterwards) might try to access the sheets.

  * If we fail to obtain a pointer to a sheet, we stop processing that
    sheet. This should not happen, and if something extremely weird
    happens it can hide the problem, but that's still better than the
    segfault we'd get from trying to operate on an invalid pointer.

  * The sheets are now removed/destroyed in reverse order, last-in
    first-out.
---

diff --git a/src/svgtiny.c b/src/svgtiny.c
index 3101493..53ced0a 100644
--- a/src/svgtiny.c
+++ b/src/svgtiny.c
@@ -801,9 +801,34 @@ cleanup:
 	const css_stylesheet* sheet;
 	css_select_ctx_count_sheets(state.select_ctx, &n_sheets);
 	for (i = 0; i < n_sheets; i++) {
-		css_code = css_select_ctx_get_sheet(state.select_ctx, i, &sheet);
-		if (css_code != CSS_OK && code == svgtiny_OK) {
-		  code = svgtiny_LIBCSS_ERROR;
+		/* Note the use of (n_sheets - 1 - i) below to
+		 * remove/destroy the sheets in reverse order, last-in
+		 * first-out. */
+		css_code = css_select_ctx_get_sheet(state.select_ctx,
+							n_sheets - 1 - i,
+							&sheet);
+		if (css_code != CSS_OK) {
+			if (code == svgtiny_OK) {
+				code = svgtiny_LIBCSS_ERROR;
+			}
+			/* If we didn't get a new sheet, don't try to
+			 * remove/destroy whatever happens to live at
+			 * the pointer. */
+			continue;
+		}
+		/* Remove the sheets from the context as well, since
+		 * there is no promise that css_select_ctx_destroy()
+		 * will not try to access them. */
+		css_code = css_select_ctx_remove_sheet(state.select_ctx, sheet);
+		if (css_code != CSS_OK) {
+			if (code == svgtiny_OK) {
+				code = svgtiny_LIBCSS_ERROR;
+			}
+			/* Don't try to destroy the sheet if removal
+			 * failed, because that would suggest that our
+			 * implied contract (that nobody else modifies
+			 * our context) has been violated. */
+			continue;
 		}
 		css_code = css_stylesheet_destroy((css_stylesheet*)sheet);
 		if (css_code != CSS_OK && code == svgtiny_OK) {