From: Frank Lichtenheld <[email protected]> - We assume that addrinfo->ai_family always fits into a sa_family_t, so we do not check those. - Reduce places where we explicitly pass ai_family and prefer sock->info.af. - Make sure that --bind-dev has a suitable length. - Treat PROTO_* as uint8_t where possible (however, much code uses proto = -1 for various error handling).
Change-Id: I7be7427480d3540d43dd818eddb6eb5860956459 Signed-off-by: Frank Lichtenheld <[email protected]> Acked-by: Razvan Cojocaru <[email protected]> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1476 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1476 This mail reflects revision 10 of this Change. Acked-by according to Gerrit (reflected above): Razvan Cojocaru <[email protected]> diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 107b3fa9..089bff4 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -5125,7 +5125,15 @@ else if (streq(p[0], "bind-dev") && p[1]) { VERIFY_PERMISSION(OPT_P_SOCKFLAGS); - options->bind_dev = p[1]; + if (strlen(p[1]) < IFNAMSIZ) + { + options->bind_dev = p[1]; + } + else + { + msg(msglevel, "argument to --bind-dev is longer than allowed %u", IFNAMSIZ - 1); + goto err; + } } #endif else if (streq(p[0], "txqueuelen") && p[1] && !p[2]) @@ -5211,11 +5219,9 @@ } else if (streq(p[0], "proto") && p[1] && !p[2]) { - int proto; - sa_family_t af; VERIFY_PERMISSION(OPT_P_GENERAL | OPT_P_CONNECTION); - proto = ascii2proto(p[1]); - af = ascii2af(p[1]); + int proto = ascii2proto(p[1]); + sa_family_t af = ascii2af(p[1]); if (proto < 0) { msg(msglevel, "Bad protocol: '%s'. Allowed protocols with --proto option: %s", p[1], diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index 0f66ad51..0a08267 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -619,28 +619,23 @@ } static void -bind_local(struct link_socket *sock, const sa_family_t ai_family) +bind_local(struct link_socket *sock) { /* bind to local address/port */ if (sock->bind_local) { if (sock->socks_proxy && sock->info.proto == PROTO_UDP) { - socket_bind(sock->ctrl_sd, sock->info.lsa->bind_local, ai_family, "SOCKS", false); + socket_bind(sock->ctrl_sd, sock->info.lsa->bind_local, sock->info.af, "SOCKS", false); } else { - socket_bind(sock->sd, sock->info.lsa->bind_local, ai_family, "TCP/UDP", + socket_bind(sock->sd, sock->info.lsa->bind_local, sock->info.af, "TCP/UDP", sock->info.bind_ipv6_only); } } } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wconversion" -#endif - static void create_socket(struct link_socket *sock, struct addrinfo *addr) { @@ -672,7 +667,7 @@ } /* Set af field of sock->info, so it always reflects the address family * of the created socket */ - sock->info.af = addr->ai_family; + sock->info.af = (sa_family_t)addr->ai_family; /* set socket buffers based on --sndbuf and --rcvbuf options */ socket_set_buffers(sock->sd, &sock->socket_buffer_sizes, true); @@ -684,8 +679,9 @@ if (sock->bind_dev) { msg(M_INFO, "Using bind-dev %s", sock->bind_dev); + /* Note: We verify strlen of bind_dev in options parsing */ if (setsockopt(sock->sd, SOL_SOCKET, SO_BINDTODEVICE, sock->bind_dev, - strlen(sock->bind_dev) + 1) + (socklen_t)(strlen(sock->bind_dev) + 1)) != 0) { msg(M_WARN | M_ERRNO, "WARN: setsockopt SO_BINDTODEVICE=%s failed", sock->bind_dev); @@ -693,13 +689,9 @@ } #endif - bind_local(sock, addr->ai_family); + bind_local(sock); } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic pop -#endif - #ifdef TARGET_ANDROID static void protect_fd_nonlocal(int fd, const struct sockaddr *addr) @@ -1131,13 +1123,8 @@ } } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wconversion" -#endif - static void -resolve_bind_local(struct link_socket *sock, const sa_family_t af) +resolve_bind_local(struct link_socket *sock) { struct gc_arena gc = gc_new(); @@ -1153,12 +1140,12 @@ } /* will return AF_{INET|INET6}from local_host */ - status = get_cached_dns_entry(sock->dns_cache, sock->local_host, sock->local_port, af, + status = get_cached_dns_entry(sock->dns_cache, sock->local_host, sock->local_port, sock->info.af, flags, &sock->info.lsa->bind_local); if (status) { - status = openvpn_getaddrinfo(flags, sock->local_host, sock->local_port, 0, NULL, af, + status = openvpn_getaddrinfo(flags, sock->local_host, sock->local_port, 0, NULL, sock->info.af, &sock->info.lsa->bind_local); } @@ -1179,7 +1166,7 @@ /* the resolved 'local entry' might have a different family than * what was globally configured */ - sock->info.af = sock->info.lsa->bind_local->ai_family; + sock->info.af = (sa_family_t)sock->info.lsa->bind_local->ai_family; } } @@ -1382,7 +1369,8 @@ sock->mark = o->mark; sock->bind_dev = o->bind_dev; - sock->info.proto = proto; + ASSERT(proto >= 0 && proto < PROTO_N); + sock->info.proto = (uint8_t)proto; sock->info.af = o->ce.af; sock->info.remote_float = o->ce.remote_float; sock->info.lsa = &c->c1.link_socket_addrs[sock_index]; @@ -1448,7 +1436,7 @@ { if (sock->bind_local) { - resolve_bind_local(sock, sock->info.af); + resolve_bind_local(sock); } resolve_remote(sock, 1, NULL); } @@ -1713,9 +1701,9 @@ * and we should not connect a remote */ if (sock->info.af == AF_UNSPEC) { + sock->info.af = (sa_family_t)sock->info.lsa->bind_local->ai_family; msg(M_WARN, "Could not determine IPv4/IPv6 protocol. Using %s", - addr_family_name(sock->info.lsa->bind_local->ai_family)); - sock->info.af = sock->info.lsa->bind_local->ai_family; + addr_family_name(sock->info.af)); } create_socket(sock, sock->info.lsa->bind_local); } @@ -1775,10 +1763,6 @@ } } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic pop -#endif - void link_socket_close(struct link_socket *sock) { @@ -2384,8 +2368,8 @@ else if (cmsg != NULL) { msg(M_WARN, - "CMSG received that cannot be parsed (cmsg_level=%d, cmsg_type=%d, cmsg=len=%d)", - (int)cmsg->cmsg_level, (int)cmsg->cmsg_type, (int)cmsg->cmsg_len); + "CMSG received that cannot be parsed (cmsg_level=%d, cmsg_type=%d, cmsg=len=%zu)", + cmsg->cmsg_level, cmsg->cmsg_type, (size_t)cmsg->cmsg_len); } return buf->len; diff --git a/src/openvpn/socket_util.c b/src/openvpn/socket_util.c index 0194f38..913ac69 100644 --- a/src/openvpn/socket_util.c +++ b/src/openvpn/socket_util.c @@ -356,7 +356,7 @@ const char *short_form; const char *display_form; sa_family_t proto_af; - int proto; + uint8_t proto; }; /* Indexed by PROTO_x */ _______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
