From 134bb0a2a38f053e372dad6233ca0c600d659b90 Mon Sep 17 00:00:00 2001 From: eugen Date: Mon, 12 Mar 2018 17:37:38 +0000 Subject: [PATCH] MFC r329105: ppp(8): fix code producing debugging logs ppp(8): fix code producing debugging logs Fix several cases when long buffer is copied to shorter one using snprintf that results in contents truncation and clobbering unsaved errno value and creation of misleading logs. PR: 218517 Approved by: mav (mentor) git-svn-id: svn://svn.freebsd.org/base/stable/10@330805 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f --- usr.sbin/ppp/defs.h | 2 ++ usr.sbin/ppp/iface.c | 24 ++++++++++++++---------- usr.sbin/ppp/ip.c | 2 +- usr.sbin/ppp/ipv6cp.c | 4 ++-- usr.sbin/ppp/ncpaddr.c | 2 -- usr.sbin/ppp/route.c | 4 ++-- 6 files changed, 21 insertions(+), 17 deletions(-) diff --git a/usr.sbin/ppp/defs.h b/usr.sbin/ppp/defs.h index d320a5372..c13fdcc08 100644 --- a/usr.sbin/ppp/defs.h +++ b/usr.sbin/ppp/defs.h @@ -117,6 +117,8 @@ #define ROUNDUP(x) ((x) ? (1 + (((x) - 1) | (sizeof(long) - 1))) : sizeof(long)) +#define NCP_ASCIIBUFFERSIZE 52 + #ifdef __NetBSD__ extern void randinit(void); #else diff --git a/usr.sbin/ppp/iface.c b/usr.sbin/ppp/iface.c index 8fee189a9..687395dce 100644 --- a/usr.sbin/ppp/iface.c +++ b/usr.sbin/ppp/iface.c @@ -209,7 +209,7 @@ iface_addr_Zap(const char *name, struct iface_addr *addr, int s) #endif struct sockaddr_in *me4, *msk4, *peer4; struct sockaddr_storage ssme, sspeer, ssmsk; - int res; + int res, saved_errno; ncprange_getsa(&addr->ifa, &ssme, &ssmsk); ncpaddr_getsa(&addr->peer, &sspeer); @@ -235,8 +235,9 @@ iface_addr_Zap(const char *name, struct iface_addr *addr, int s) memcpy(peer4, &sspeer, sizeof *peer4); res = ID0ioctl(s, SIOCDIFADDR, &ifra); + saved_errno = errno; if (log_IsKept(LogDEBUG)) { - char buf[100]; + char buf[NCP_ASCIIBUFFERSIZE]; snprintf(buf, sizeof buf, "%s", ncprange_ntoa(&addr->ifa)); log_Printf(LogWARN, "%s: DIFADDR %s -> %s returns %d\n", @@ -260,12 +261,13 @@ iface_addr_Zap(const char *name, struct iface_addr *addr, int s) ifra6.ifra_lifetime.ia6t_pltime = ND6_INFINITE_LIFETIME; res = ID0ioctl(s, SIOCDIFADDR_IN6, &ifra6); + saved_errno = errno; break; #endif } if (res == -1) { - char dst[40]; + char dst[NCP_ASCIIBUFFERSIZE]; const char *end = #ifndef NOINET6 ncprange_family(&addr->ifa) == AF_INET6 ? "_IN6" : @@ -274,11 +276,11 @@ iface_addr_Zap(const char *name, struct iface_addr *addr, int s) if (ncpaddr_family(&addr->peer) == AF_UNSPEC) log_Printf(LogWARN, "iface rm: ioctl(SIOCDIFADDR%s, %s): %s\n", - end, ncprange_ntoa(&addr->ifa), strerror(errno)); + end, ncprange_ntoa(&addr->ifa), strerror(saved_errno)); else { snprintf(dst, sizeof dst, "%s", ncpaddr_ntoa(&addr->peer)); log_Printf(LogWARN, "iface rm: ioctl(SIOCDIFADDR%s, %s -> %s): %s\n", - end, ncprange_ntoa(&addr->ifa), dst, strerror(errno)); + end, ncprange_ntoa(&addr->ifa), dst, strerror(saved_errno)); } } @@ -294,7 +296,7 @@ iface_addr_Add(const char *name, struct iface_addr *addr, int s) #endif struct sockaddr_in *me4, *msk4, *peer4; struct sockaddr_storage ssme, sspeer, ssmsk; - int res; + int res, saved_errno; ncprange_getsa(&addr->ifa, &ssme, &ssmsk); ncpaddr_getsa(&addr->peer, &sspeer); @@ -320,8 +322,9 @@ iface_addr_Add(const char *name, struct iface_addr *addr, int s) memcpy(peer4, &sspeer, sizeof *peer4); res = ID0ioctl(s, SIOCAIFADDR, &ifra); + saved_errno = errno; if (log_IsKept(LogDEBUG)) { - char buf[100]; + char buf[NCP_ASCIIBUFFERSIZE]; snprintf(buf, sizeof buf, "%s", ncprange_ntoa(&addr->ifa)); log_Printf(LogWARN, "%s: AIFADDR %s -> %s returns %d\n", @@ -345,12 +348,13 @@ iface_addr_Add(const char *name, struct iface_addr *addr, int s) ifra6.ifra_lifetime.ia6t_pltime = ND6_INFINITE_LIFETIME; res = ID0ioctl(s, SIOCAIFADDR_IN6, &ifra6); + saved_errno = errno; break; #endif } if (res == -1) { - char dst[40]; + char dst[NCP_ASCIIBUFFERSIZE]; const char *end = #ifndef NOINET6 ncprange_family(&addr->ifa) == AF_INET6 ? "_IN6" : @@ -359,11 +363,11 @@ iface_addr_Add(const char *name, struct iface_addr *addr, int s) if (ncpaddr_family(&addr->peer) == AF_UNSPEC) log_Printf(LogWARN, "iface add: ioctl(SIOCAIFADDR%s, %s): %s\n", - end, ncprange_ntoa(&addr->ifa), strerror(errno)); + end, ncprange_ntoa(&addr->ifa), strerror(saved_errno)); else { snprintf(dst, sizeof dst, "%s", ncpaddr_ntoa(&addr->peer)); log_Printf(LogWARN, "iface add: ioctl(SIOCAIFADDR%s, %s -> %s): %s\n", - end, ncprange_ntoa(&addr->ifa), dst, strerror(errno)); + end, ncprange_ntoa(&addr->ifa), dst, strerror(saved_errno)); } } diff --git a/usr.sbin/ppp/ip.c b/usr.sbin/ppp/ip.c index 5c25a0947..88f9ffd66 100644 --- a/usr.sbin/ppp/ip.c +++ b/usr.sbin/ppp/ip.c @@ -224,7 +224,7 @@ FilterCheck(const unsigned char *packet, int match; /* true if condition matched */ int mindata; /* minimum data size or zero */ const struct filterent *fp = filter->rule; - char dbuff[100], dstip[16]; + char dbuff[100], dstip[NCP_ASCIIBUFFERSIZE]; struct ncpaddr srcaddr, dstaddr; const char *payload; /* IP payload */ int datalen; /* IP datagram length */ diff --git a/usr.sbin/ppp/ipv6cp.c b/usr.sbin/ppp/ipv6cp.c index 72c3b9a86..e33e79a35 100644 --- a/usr.sbin/ppp/ipv6cp.c +++ b/usr.sbin/ppp/ipv6cp.c @@ -465,7 +465,7 @@ ipv6cp_LayerUp(struct fsm *fp) { /* We're now up */ struct ipv6cp *ipv6cp = fsm2ipv6cp(fp); - char tbuff[40]; + char tbuff[NCP_ASCIIBUFFERSIZE]; log_Printf(LogIPV6CP, "%s: LayerUp.\n", fp->link->name); if (!ipv6cp_InterfaceUp(ipv6cp)) @@ -522,7 +522,7 @@ ipv6cp_LayerDown(struct fsm *fp) /* About to come down */ struct ipv6cp *ipv6cp = fsm2ipv6cp(fp); static int recursing; - char addr[40]; + char addr[NCP_ASCIIBUFFERSIZE]; if (!recursing++) { snprintf(addr, sizeof addr, "%s", ncpaddr_ntoa(&ipv6cp->myaddr)); diff --git a/usr.sbin/ppp/ncpaddr.c b/usr.sbin/ppp/ncpaddr.c index 0b1699939..7372dc200 100644 --- a/usr.sbin/ppp/ncpaddr.c +++ b/usr.sbin/ppp/ncpaddr.c @@ -76,8 +76,6 @@ #define ncpaddr_ip6addr u.ip6addr #endif -#define NCP_ASCIIBUFFERSIZE 52 - static struct in_addr bits2mask4(int bits) { diff --git a/usr.sbin/ppp/route.c b/usr.sbin/ppp/route.c index 758b40319..489e6cf80 100644 --- a/usr.sbin/ppp/route.c +++ b/usr.sbin/ppp/route.c @@ -435,7 +435,7 @@ route_IfDelete(struct bundle *bundle, int all) ) && (all || (rtm->rtm_flags & RTF_GATEWAY))) { if (log_IsKept(LogDEBUG)) { - char gwstr[41]; + char gwstr[NCP_ASCIIBUFFERSIZE]; struct ncpaddr gw; ncprange_setsa(&range, sa[RTAX_DST], sa[RTAX_NETMASK]); ncpaddr_setsa(&gw, sa[RTAX_GATEWAY]); @@ -840,7 +840,7 @@ failed: } if (log_IsKept(LogDEBUG)) { - char gwstr[40]; + char gwstr[NCP_ASCIIBUFFERSIZE]; if (gw) snprintf(gwstr, sizeof gwstr, "%s", ncpaddr_ntoa(gw)); -- 2.42.0