BUG/MINOR: server: fix first server template name lookup UAF
authorAurelien DARRAGON <adarragon@haproxy.com>
Thu, 27 Jun 2024 08:42:44 +0000 (10:42 +0200)
committerChristopher Faulet <cfaulet@haproxy.com>
Wed, 3 Jul 2024 06:43:17 +0000 (08:43 +0200)
commite3385b697e2b28d5107a6d37cf1abb197d9b15c3
tree3e36b9d18cf74ec3116b646d4225f3dbc63965d8
parentaeb5cbdb23c93c21583f9f88bae3c8950130e4f9
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.

(cherry picked from commit eec804804212374739556175f81b234d7cc8c6f0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
src/server.c