mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-09-22 22:31:28 +02:00
Code Issues Projects Releases Wiki Activity

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

Browse Source 
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)
This commit is contained in:
Aurelien DARRAGON 2023-01-30 09:28:57 +01:00 committed by Christopher Faulet
parent c254516c53
commit 8436c910f5
1 changed files with 23 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
src/http_ext.c
View File

@ -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)) {