Thanks for the guidance, Rodrigo! Had a better idea on how to fix the socketdata use-after-free; new attached patch 0002 fixes the bug by only freeing the queued sockdata if it's exclusively owned by the server queue.
I've added information in the commit summaries as suggested. I also added an assert in Http_socket_enqueue, which makes explicit the precondition that socketdata can only be queued to one server at a time. This assert has not tripped, and hopefully never will; I thought it helped illustrate the relation between servers and socketdatas. Magnus On Sat, Jan 24, 2026 at 2:10 PM Rodrigo Arias <[email protected]> wrote: > > Hi Magnus, > > On Sat, Jan 24, 2026 at 11:13:25AM -0800, Nopey Nope wrote: > >Hello! > > > >Find attached two patches, which each fix a use-after-free bug. > > > >I've also attached a crashing html document for each, but please be > >aware that these reproducer documents link urls from external web > >servers. > >The reproducers crash reliably on my x86_64 Void Linux laptop, but > >I've not tested them elsewhere. I've also attached a crash log with > >asan reporting of each. > > > >Thank you > >-Magnus L > > > >p.s. I've also opened github PR 449 with these same changes, but > >figured I should send the patches via email as well-- if only for > >practice :) > > Thanks a lot for the patches and reproducers! > > I saw the PR but I only had time to look at the "Fix use-after-free in > openssl cert popup" commit which seems good. We are moving away from > GitHub so it would be nice to send future patches here (or a link to a > git repo/branch to fetch with git). > > It would help to describe what is the condition that triggers the UAF in > the Http_server patch and why the change helps, so that it is easier to > review. > > Perhaps it would be a good idea to put that information it in the commit > summary of both patches so we can see it via git blame. > > Best, > Rodrigo. > _______________________________________________ > Dillo-dev mailing list -- [email protected] > To unsubscribe send an email to [email protected]
From 239b0d2dc72c52251ac9cdf371f4567faadea9a4 Mon Sep 17 00:00:00 2001 From: Magnus Larsen <[email protected]> Date: Thu, 22 Jan 2026 23:57:28 -0800 Subject: [PATCH 1/2] Fix use-after-free in openssl cert popup During Tls_connect, we allow FLTK to run callbacks when we run the certificate verification dialog. Some callbacks free connections, such as Nav_repush_callback. We must check our conn (identified by connkey) has not been freed thusly by looking it up in the valid_conns klist before writing to any fields. The conn->in_connect write was missing such a check, and so would write into freed memory. This write is specific to the tls_openssl port, and so embedtls is not effected. --- src/IO/tls_openssl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/IO/tls_openssl.c b/src/IO/tls_openssl.c index dddde0e8..dd619996 100644 --- a/src/IO/tls_openssl.c +++ b/src/IO/tls_openssl.c @@ -1213,8 +1213,10 @@ static void Tls_connect(int fd, int connkey) * been closed by the server if the user responded too slowly to a popup. */ + conn = a_Klist_get_data(conn_list, connkey); + if (!ongoing) { - if (a_Klist_get_data(conn_list, connkey)) { + if (conn) { conn->connecting = FALSE; if (failed) { conn->in_connect = FALSE; -- 2.51.2
From f23178c2d8fb903bc55cc2fb38787ce2cfb1b13e Mon Sep 17 00:00:00 2001 From: Magnus Larsen <[email protected]> Date: Sat, 24 Jan 2026 20:04:55 -0700 Subject: [PATCH 2/2] Fix premature SocketData free in Http_server_remove SocketData * (hereafter, SD) are kept in two places: the ValidSocks klist (where they are added immediately upon creation), and sometimes in a Server_t's queue dlist. The bug occurs when the following events happen in sequence: 1. Http_sock_new creates a new SD, and adds it to the ValidSocks list. 2. The SD is queued to a server. 3. The server is removed via Http_server_remove or Http_servers_remove_all. The buggy implementation unconditionally dFrees all SD's queued to the server, leaving dangling pointers in the ValidSocks klist. 4. The SD's SKey is no longer needed, so Http_socket_free is called. Http_socket_free reads the SD pointer from ValidSocks, and reads the flags from the dangling pointer (ka-boom). This commit modifies step 3 to only free SD that is no longer valid (no longer inside of klist ValidSocks) by checking for flag HTTP_SOCKET_TO_BE_FREED. --- src/IO/http.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/IO/http.c b/src/IO/http.c index 4d9902f2..8ec07140 100644 --- a/src/IO/http.c +++ b/src/IO/http.c @@ -1045,6 +1045,8 @@ void a_Http_ccc(int Op, int Branch, int Dir, ChainLink *Info, */ static void Http_socket_enqueue(Server_t *srv, SocketData_t* sock) { + assert(("sock cannot belong to more than one queue", (sock->flags & HTTP_SOCKET_QUEUED) == 0)); + sock->flags |= HTTP_SOCKET_QUEUED; if ((sock->web->flags & WEB_Image) == 0) { @@ -1092,7 +1094,9 @@ static void Http_server_remove(Server_t *srv) while ((sd = dList_nth_data(srv->queue, 0))) { dList_remove_fast(srv->queue, sd); - dFree(sd); + sd->flags &= ~HTTP_SOCKET_QUEUED; + if (sd->flags & HTTP_SOCKET_TO_BE_FREED) + dFree(sd); } dList_free(srv->queue); dList_remove_fast(servers, srv); @@ -1108,8 +1112,10 @@ static void Http_servers_remove_all(void) while (dList_length(servers) > 0) { srv = (Server_t*) dList_nth_data(servers, 0); while ((sd = dList_nth_data(srv->queue, 0))) { - dList_remove(srv->queue, sd); - dFree(sd); + dList_remove_fast(srv->queue, sd); + sd->flags &= ~HTTP_SOCKET_QUEUED; + if (sd->flags & HTTP_SOCKET_TO_BE_FREED) + dFree(sd); } Http_server_remove(srv); } -- 2.51.2
_______________________________________________ Dillo-dev mailing list -- [email protected] To unsubscribe send an email to [email protected]
