BUG/MINOR: server: fix first server template name lookup UAF

This is a follow-up for 7223296 ("BUG/MINOR: server: fix first server
template not being indexed").

Indeed, in 7223296 we added a new call to _srv_parse_set_id_from_prefix()
for the first server before handling additional ones. But we actually
overlooked the fact that _srv_parse_set_id_from_prefix() was already
performed at the end of _srv_parse_tmpl_init() for the same server.

Since _srv_parse_set_id_from_prefix() frees srv->id, it results in UAF
when performing name lookups on the first server, because used_server_name
node key still uses the freed string pointer.

The early _srv_parse_set_id_from_prefix() call (added in 7223296) and
the original one perform the same task, except that the new one is
followed by name node insertion logic required for name lookups to work
properly. So let's simply get rid of the old one at the end of the
function.

_srv_parse_set_id_from_prefix() in the 'err:' label was also removed since
is is now useless as well starting with 7223296 and would trigger the same
bug on error paths. Thanks to Amaury for noticing it.

This bug was discovered while trying to address GH issue #2620.
Thanks to @x-yuri for his detailed report (with working repro).

It should be backported in 3.0 with 7223296.

diff --git a/src/server.c b/src/server.c
index caf2f40bb537cf0cfb55ecf9390d630dbe9f7b0d..d5c6318749c7cbb0512be1e83f28747e389c252b 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -3177,12 +3177,10 @@ static int _srv_parse_tmpl_init(struct server *srv, struct proxy *px)
                newsrv->conf.name.key = newsrv->id;
                ebis_insert(&curproxy->conf.used_server_name, &newsrv->conf.name);
        }
-       _srv_parse_set_id_from_prefix(srv, srv->tmpl_info.prefix, srv->tmpl_info.nb_low);
 
        return i - srv->tmpl_info.nb_low;
 
  err:
-       _srv_parse_set_id_from_prefix(srv, srv->tmpl_info.prefix, srv->tmpl_info.nb_low);
        if (newsrv)  {
                release_sample_expr(newsrv->ssl_ctx.sni);
                free_check(&newsrv->agent);