From 882f977ef1643dfb07f57b91ae69e58d9817936f Mon Sep 17 00:00:00 2001 From: Acts1631 Date: Tue, 7 Jul 2026 22:06:56 -0400 Subject: [PATCH] security: fix buffer overflow and HTML injection in bookmark export Five sprintf() calls wrote filenames into a 256-byte stack buffer using data->filename, which comes from gtk_file_chooser_get_filename() and can be up to PATH_MAX (4096 on Linux). A path longer than 251 chars overflowed the buffer when the extension was appended. The affected functions are _save_verselist_2_xml, _save_verselist_2_html (both .html and .txt branches), save_iter_to_xml, and _save_iter (both branches). export_2_html also had a dead sprintf into a 256-byte buffer whose result was never read but still corrupted the stack. Replaced all fixed buffers with g_strdup_printf() allocations and freed them after use; removed the dead code in export_2_html. Bookmark description, module, key, folder caption, and search-result title were inserted verbatim into exported HTML output, allowing stored XSS when the exported file is opened in a browser. A bookmark description like would execute on view. Escape these user-controlled values with g_markup_escape_text() before formatting them into HTML. The affected locations are gui_set_html_item, _parse_treeview (folder caption), _save_iter (HTML header caption), and _save_verselist_2_html (search result title). Rendered scripture text is left unescaped, as it is legitimate HTML produced by the SWORD render filters. --- src/gtk/export_bookmarks.c | 63 +++++++++++++++++++++++++------------- 1 file changed, 42 insertions(+), 21 deletions(-) diff --git a/src/gtk/export_bookmarks.c b/src/gtk/export_bookmarks.c index 635f4560f..18e02aa17 100644 --- a/src/gtk/export_bookmarks.c +++ b/src/gtk/export_bookmarks.c @@ -102,9 +102,19 @@ void gui_set_html_item(GString *str, } else scripture = g_strdup(""); - buf = g_strdup_printf("
  • %s
    • ", - description, - module, key, (scripture ? scripture : "")); + { + gchar *desc_esc = g_markup_escape_text(description, -1); + gchar *mod_esc = g_markup_escape_text(module, -1); + gchar *key_esc = g_markup_escape_text(key, -1); + + buf = g_strdup_printf("
  • %s
    • ", + desc_esc, + mod_esc, key_esc, (scripture ? scripture : "")); + + g_free(desc_esc); + g_free(mod_esc); + g_free(key_esc); + } g_string_append(str, buf); g_free(buf); @@ -152,11 +162,11 @@ static gboolean _save_verselist_2_xml(BK_EXPORT *data) xmlNodePtr cur_node = NULL; xmlDocPtr root_doc; //xmlAttrPtr root_attr; - gchar filename[256]; + gchar *filename; GString *name = g_string_new(NULL); GString *str = g_string_new(""); - sprintf(filename, "%s.xml", data->filename); + filename = g_strdup_printf("%s.xml", data->filename); root_doc = xmlNewDoc((const xmlChar *)"1.0"); @@ -191,12 +201,13 @@ static gboolean _save_verselist_2_xml(BK_EXPORT *data) xmlSaveFormatFile(filename, root_doc, 1); xmlFreeDoc(root_doc); + g_free(filename); return 1; } static gboolean _save_verselist_2_html(BK_EXPORT *data) { - gchar filename[256]; + gchar *filename = NULL; GString *name = g_string_new(NULL); GString *str = g_string_new(""); GString *des = g_string_new(""); @@ -209,11 +220,11 @@ static gboolean _save_verselist_2_html(BK_EXPORT *data) switch (data->type) { case HTML: catenate = gui_set_html_item; - sprintf(filename, "%s.html", data->filename); + filename = g_strdup_printf("%s.html", data->filename); break; case PLAIN: catenate = gui_set_plain_text_item; - sprintf(filename, "%s.txt", data->filename); + filename = g_strdup_printf("%s.txt", data->filename); break; } @@ -232,11 +243,14 @@ static gboolean _save_verselist_2_html(BK_EXPORT *data) g_string_printf(name, _("Verse List")); switch (data->type) { - case HTML: + case HTML: { + gchar *name_esc = g_markup_escape_text(name->str, -1); buf = g_strdup_printf("