@@ -324,8 +324,14 @@ verify_auth_token(struct user_pass *up, struct tls_multi *multi,
const uint8_t *tstamp_initial = sessid + AUTH_TOKEN_SESSION_ID_LEN;
const uint8_t *tstamp = tstamp_initial + sizeof(int64_t);
- uint64_t timestamp = ntohll(*((uint64_t *) (tstamp)));
- uint64_t timestamp_initial = ntohll(*((uint64_t *) (tstamp_initial)));
+ /* This might not be aligned to an uint64, use memcpy to avoid
+ * unaligned access */
+ uint64_t timestamp = 0, timestamp_initial = 0;
+ memcpy(×tamp, tstamp, sizeof(uint64_t));
+ timestamp = ntohll(timestamp);
+
+ memcpy(×tamp_initial, tstamp_initial, sizeof(uint64_t));
+ timestamp_initial = ntohll(timestamp_initial);
hmac_ctx_t *ctx = multi->opt.auth_token_key.hmac;
if (check_hmac_token(ctx, b64decoded, up->username))
@@ -3637,7 +3637,7 @@ get_default_gateway(struct route_gateway_info *rgi, openvpn_net_ctx_t *ctx)
if (rgi->flags & RGI_IFACE_DEFINED)
{
struct ifconf ifc;
- struct ifreq *ifr;
+ struct ifreq ifr;
const int bufsize = 4096;
char *buffer;
@@ -3662,23 +3662,37 @@ get_default_gateway(struct route_gateway_info *rgi, openvpn_net_ctx_t *ctx)
for (cp = buffer; cp <= buffer + ifc.ifc_len - sizeof(struct ifreq); )
{
- ifr = (struct ifreq *)cp;
+ /* this is not always using an 8byte alignment that struct ifr
+ * requires */
+ memcpy(&ifr, cp, sizeof(struct ifreq));
#if defined(TARGET_SOLARIS)
- const size_t len = sizeof(ifr->ifr_name) + sizeof(ifr->ifr_addr);
+ const size_t len = sizeof(ifr.ifr_name) + sizeof(ifr.ifr_addr);
#else
- const size_t len = sizeof(ifr->ifr_name) + max(sizeof(ifr->ifr_addr), ifr->ifr_addr.sa_len);
+ const size_t len = sizeof(ifr.ifr_name) + max(sizeof(ifr.ifr_addr), ifr.ifr_addr.sa_len);
#endif
- if (!ifr->ifr_addr.sa_family)
+ if (!ifr.ifr_addr.sa_family)
{
break;
}
- if (!strncmp(ifr->ifr_name, rgi->iface, IFNAMSIZ))
+ if (!strncmp(ifr.ifr_name, rgi->iface, IFNAMSIZ))
{
- if (ifr->ifr_addr.sa_family == AF_LINK)
+ if (ifr.ifr_addr.sa_family == AF_LINK)
{
- struct sockaddr_dl *sdl = (struct sockaddr_dl *)&ifr->ifr_addr;
- memcpy(rgi->hwaddr, LLADDR(sdl), 6);
+ /* This is a broken member access. struct sockaddr_dl has
+ * 20 bytes while if_addr has only 16 bytes. So casting if_addr
+ * to struct sockaddr_dl gives (legitimate) warnings
+ *
+ * sockaddr_dl has 12 bytes space for the hw address and
+ * Ethernet only uses 6 bytes. So the last 4 that are
+ * truncated and not in if_addr can be ignored here.
+ *
+ * So we use a memcpy here to avoid the warnings with ASAN
+ * that we are doing a very nasty cast here
+ */
+ struct sockaddr_dl sdl = { 0 };
+ memcpy(&sdl, &ifr.ifr_addr, sizeof(ifr.ifr_addr));
+ memcpy(rgi->hwaddr, LLADDR(&sdl), 6);
rgi->flags |= RGI_HWADDR_DEFINED;
}
}
The undefined behaviour USAN clang checker found these two cases. The optimiser of clang/gcc will optimise the memcpy away in the auth_token case and output excactly the same assembly on amd64/arm64 but it is still better to not rely on undefined behaviour. The hw addr fix is a mess but so are the original structures. Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/auth_token.c | 10 ++++++++-- src/openvpn/route.c | 32 +++++++++++++++++++++++--------- 2 files changed, 31 insertions(+), 11 deletions(-)