BUG/MINOR: http_ext/7239: ipv6 dumping relies on out of scope variables

In http_build_7239_header_nodename(), ip6 address dumping is performed
at a single place to prevent code duplication:
A goto statement combined with a local pointer variable (ip6_addr) were used
to perform ipv6 dump from different calling places inside the function.

However, when the goto was performed (ie: sample expression handling),
ip6_addr pointer was assigned to limited scope variable's address that is not
valid within the dumping code.
Because of this, we have an undefined behavior that could result in a bug or
a crash depending on the platform that is running haproxy.

This was found by Coverity (GH #2018)

To fix this, we add a simple ip6 printing helper that takes the ip6_addr
pointer as an argument.
This prevents any scope related bug as the function is executed under the
proper context.

if/else guards inside the function were reviewed to make sure that the goto
removal won't affect existing behavior.

----------

No backport needed, except if the commit ("MINOR: proxy/http_ext: introduce
proxy forwarded option") is backported.

Given that this commit needs to be backported with
"MINOR: proxy/http_ext: introduce proxy forwarded option", We're using it as a
reminder for another bug that was introduced with
"MINOR: proxy/http_ext: introduce proxy forwarded option" but has been silently
fixed since with
"MEDIUM: proxy/http_ext: implement dynamic http_ext".

If "MINOR: proxy/http_ext: introduce proxy forwarded option" needs to be
backported without
"MEDIUM: proxy/http_ext: implement dynamic http_ext", you should manually apply
the following patch on top of it:

    |  diff --git a/src/http_ext.c b/src/http_ext.c
    |  index fcb5a07bc..3921357a3 100644
    |  --- a/src/http_ext.c
    |  +++ b/src/http_ext.c
    |  @@ -609,7 +609,7 @@ static inline void http_build_7239_header_node(struct buffer *out,
    |   	if (forby->np_mode)
    |   		chunk_appendf(out, "\"");
    |   	offset_save = out->data;
    |  -	http_build_7239_header_node(out, s, curproxy, addr, &curproxy->http.fwd.p_by);
    |  +	http_build_7239_header_nodename(out, s, curproxy, addr, forby);
    |   	if (offset_save == out->data) {
    |   		/* could not build nodename, either because some
    |   		 * data is not available or user is providing bad input
    |  @@ -619,7 +619,7 @@ static inline void http_build_7239_header_node(struct buffer *out,
    |   	if (forby->np_mode) {
    |   		chunk_appendf(out, ":");
    |   		offset_save = out->data;
    |  -		http_build_7239_header_nodeport(out, s, curproxy, addr, &curproxy->http.fwd.p_by);
    |  +		http_build_7239_header_nodeport(out, s, curproxy, addr, forby);
    |   		if (offset_save == out->data) {
    |   			/* could not build nodeport, either because some data is
    |   			 * not available or user is providing bad input

(If you don't, forwarded option won't work properly and will crash
haproxy (stack overflow) when building 'for' or 'by' parameter)

diff --git a/src/http_ext.c b/src/http_ext.c
index d3b57645b283cd02dd5ede7a58fe62fda54cc2c1..3875d3b748d7b614b2592ddd7e197b461c2eb08e 100644 (file)
--- a/src/http_ext.c
+++ b/src/http_ext.c
@@ -485,12 +485,25 @@ int http_validate_7239_header(struct ist hdr, int required_steps, struct forward
        return 0;
 }
 
+static inline void _7239_print_ip6(struct buffer *out, struct in6_addr *ip6_addr, int quoted)
+{
+       char pn[INET6_ADDRSTRLEN];
+
+       inet_ntop(AF_INET6,
+                 ip6_addr,
+                 pn, sizeof(pn));
+       if (!quoted)
+               chunk_appendf(out, "\""); /* explicit quoting required for ipv6 */
+       chunk_appendf(out, "[%s]", pn);
+}
+
 static inline void http_build_7239_header_nodename(struct buffer *out,
                                                    struct stream *s, struct proxy *curproxy,
                                                    const struct sockaddr_storage *addr,
                                                    struct http_ext_7239_forby *forby)
 {
        struct in6_addr *ip6_addr;
+       int quoted = !!forby->np_mode;
 
        if (forby->nn_mode == HTTP_7239_FORBY_ORIG) {
                if (addr && addr->ss_family == AF_INET) {
@@ -500,17 +513,7 @@ static inline void http_build_7239_header_nodename(struct buffer *out,
                }
                else if (addr && addr->ss_family == AF_INET6) {
                        ip6_addr = &((struct sockaddr_in6 *)addr)->sin6_addr;
- print_ip6:
-                       {
-                               char pn[INET6_ADDRSTRLEN];
-
-                               inet_ntop(AF_INET6,
-                                         ip6_addr,
-                                         pn, sizeof(pn));
-                               if (!forby->np_mode)
-                                       chunk_appendf(out, "\""); /* explicit quoting required for ipv6 */
-                               chunk_appendf(out, "[%s]", pn);
-                       }
+                       _7239_print_ip6(out, ip6_addr, quoted);
                }
                /* else: not supported */
        }
@@ -524,10 +527,10 @@ static inline void http_build_7239_header_nodename(struct buffer *out,
                        if (smp->data.type == SMP_T_IPV6) {
                                /* smp is valid IP6, print with RFC compliant output */
                                ip6_addr = &smp->data.u.ipv6;
-                               goto print_ip6;
+                               _7239_print_ip6(out, ip6_addr, quoted);
                        }
-                       if (sample_casts[smp->data.type][SMP_T_STR] &&
-                           sample_casts[smp->data.type][SMP_T_STR](smp)) {
+                       else if (sample_casts[smp->data.type][SMP_T_STR] &&
+                                sample_casts[smp->data.type][SMP_T_STR](smp)) {
                                struct ist validate_n = ist2(smp->data.u.str.area, smp->data.u.str.data);
                                struct ist validate_o = ist2(smp->data.u.str.area, smp->data.u.str.data);
                                struct forwarded_header_nodename nodename;
@@ -539,12 +542,13 @@ static inline void http_build_7239_header_nodename(struct buffer *out,
                                            nodename.ip.ss_family == AF_INET6) {
                                                /* special care needed for valid ip6 nodename (quoting) */
                                                ip6_addr = &((struct sockaddr_in6 *)&nodename.ip)->sin6_addr;
-                                               goto print_ip6;
+                                               _7239_print_ip6(out, ip6_addr, quoted);
+                                       } else {
+                                               /* no special care needed, input is already rfc compliant,
+                                                * just print as regular non quoted string
+                                                */
+                                               chunk_cat(out, &smp->data.u.str);
                                        }
-                                       /* no special care needed, input is already rfc compliant,
-                                        * just print as regular non quoted string
-                                        */
-                                       chunk_cat(out, &smp->data.u.str);
                                }
                                else if (http_7239_extract_obfs(&validate_o, NULL) &&
                                         !istlen(validate_o)) {