BUG/MEDIUM: server: Duplicate healthcheck's alpn inherited from default server

When "check-alpn" parameter is inherited from the default server, the value
is not duplicated, the pointer of the default server is used. However, when
this parameter is overridden, the old value is released. So the "check-alpn"
value of the default server is released. So it is possible to have a UAF if
if another server inherit from the same the default server.

To fix the issue, the "check-alpn" parameter must be handled the same way
the "alpn" is. The default value is duplicated. So it could be safely
released if it is forced on the server line.

This patch should fix the issue #3096. It must be backported to all stable
versions.

(cherry picked from commit f7a04b428a47edeabfe40bf1a48a85b03c105633)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 361ae50063941df0d7269489646fc4e5978ea97f)
[cf: ctx adjt]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 015732185bee1f75a3160a8a99369511a29c7aa4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

diff --git a/src/check.c b/src/check.c
index 415be90..ec0cb73 100644 (file)
--- a/src/check.c
+++ b/src/check.c
@@ -1579,6 +1579,7 @@ void free_check(struct check *check)
                ha_free(&check->tcpcheck_rules);
        }
 
+       ha_free(&check->alpn_str);
        task_destroy(check->task);
 
        check_release_buf(check, &check->bi);

diff --git a/src/server.c b/src/server.c
index 65603f0..c7fa089 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -2771,8 +2771,15 @@ void srv_settings_cpy(struct server *srv, const struct server *src, int srv_tmpl
        srv->check.use_ssl            = src->check.use_ssl;
        srv->check.port               = src->check.port;
        srv->check.sni                = src->check.sni;
-       srv->check.alpn_str           = src->check.alpn_str;
-       srv->check.alpn_len           = src->check.alpn_len;
+       if (src->check.alpn_str) {
+               srv->check.alpn_str = malloc(src->check.alpn_len);
+               if (srv->check.alpn_str) {
+                       memcpy(srv->check.alpn_str, src->check.alpn_str,
+                              src->check.alpn_len);
+                       srv->check.alpn_len = src->check.alpn_len;
+               }
+       }
+
        /* Note: 'flags' field has potentially been already initialized. */
        srv->flags                   |= src->flags;
        srv->do_check                 = src->do_check;