From 984c17894ad9ef26bd383e6d1025e559ebdfe142 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaroslav=20=C5=A0karvada?= Date: Mon, 3 Dec 2018 22:28:51 +0100 Subject: [PATCH] Fixed some issues found by coverity scan --- ppp-2.4.7-coverity-scan-fixes.patch | 453 ++++++++++++++++++++++++++++ ppp.spec | 6 +- 2 files changed, 458 insertions(+), 1 deletion(-) create mode 100644 ppp-2.4.7-coverity-scan-fixes.patch diff --git a/ppp-2.4.7-coverity-scan-fixes.patch b/ppp-2.4.7-coverity-scan-fixes.patch new file mode 100644 index 0000000..4f61a27 --- /dev/null +++ b/ppp-2.4.7-coverity-scan-fixes.patch @@ -0,0 +1,453 @@ +diff --git a/chat/chat.c b/chat/chat.c +index 710dba9..bf10733 100644 +--- a/chat/chat.c ++++ b/chat/chat.c +@@ -512,6 +512,7 @@ void msgf __V((const char *fmt, ...)) + syslog(LOG_INFO, "%s", line); + if (to_stderr) + fprintf(stderr, "%s\n", line); ++ va_end(args); + } + + /* +@@ -537,6 +538,7 @@ void fatal __V((int code, const char *fmt, ...)) + syslog(LOG_ERR, "%s", line); + if (to_stderr) + fprintf(stderr, "%s\n", line); ++ va_end(args); + terminate(code); + } + +diff --git a/pppd/auth.c b/pppd/auth.c +index 656ffe9..9a7e32d 100644 +--- a/pppd/auth.c ++++ b/pppd/auth.c +@@ -464,6 +464,7 @@ setupapfile(argv) + euid = geteuid(); + if (seteuid(getuid()) == -1) { + option_error("unable to reset uid before opening %s: %m", fname); ++ free(fname); + return 0; + } + ufile = fopen(fname, "re"); +@@ -471,6 +472,7 @@ setupapfile(argv) + fatal("unable to regain privileges: %m"); + if (ufile == NULL) { + option_error("unable to open user login data file %s", fname); ++ free(fname); + return 0; + } + check_access(ufile, fname); +@@ -481,6 +483,7 @@ setupapfile(argv) + || fgets(p, MAXSECRETLEN - 1, ufile) == NULL) { + fclose(ufile); + option_error("unable to read user login data file %s", fname); ++ free(fname); + return 0; + } + fclose(ufile); +@@ -502,6 +505,7 @@ setupapfile(argv) + explicit_passwd = 1; + } + ++ free(fname); + return (1); + } + +diff --git a/pppd/eap-tls.c b/pppd/eap-tls.c +index 1b79abf..f7f42fd 100644 +--- a/pppd/eap-tls.c ++++ b/pppd/eap-tls.c +@@ -693,6 +693,7 @@ int eaptls_init_ssl_server(eap_state * esp) + } + + strncpy(ets->peer, esp->es_server.ea_peer, MAXWORDLEN); ++ ets->peer[MAXWORDLEN - 1] = 0; + + dbglog( "getting eaptls secret" ); + if (!get_eaptls_secret(esp->es_unit, esp->es_server.ea_peer, +@@ -780,7 +781,10 @@ int eaptls_init_ssl_client(eap_state * esp) + * verify + */ + if (esp->es_client.ea_peer) ++ { + strncpy(ets->peer, esp->es_client.ea_peer, MAXWORDLEN); ++ ets->peer[MAXWORDLEN - 1] = 0; ++ } + else + ets->peer[0] = 0; + +@@ -835,7 +839,10 @@ int eaptls_init_ssl_client(eap_state * esp) + * ssl_verify_callback() + */ + if (servcertfile[0]) ++ { + strncpy(ets->peercertfile, servcertfile, MAXWORDLEN); ++ ets->peercertfile[MAXWORDLEN - 1] = 0; ++ } + else + ets->peercertfile[0] = 0; + +diff --git a/pppd/multilink.c b/pppd/multilink.c +index 2f0ed50..67200ba 100644 +--- a/pppd/multilink.c ++++ b/pppd/multilink.c +@@ -445,9 +445,13 @@ get_default_epdisc(ep) + if (p != 0 && get_if_hwaddr(ep->value, p) >= 0) { + ep->class = EPD_MAC; + ep->length = 6; ++ free(p); + return 1; + } + ++ if (p) ++ free(p); ++ + /* see if our hostname corresponds to a reasonable IP address */ + hp = gethostbyname(hostname); + if (hp != NULL) { +diff --git a/pppd/options.c b/pppd/options.c +index 1cef314..bc264d6 100644 +--- a/pppd/options.c ++++ b/pppd/options.c +@@ -1735,7 +1735,7 @@ user_unsetenv(argv) + option_error("unexpected = in name: %s", arg); + return 0; + } +- if (arg == '\0') { ++ if (*arg == '\0') { + option_error("missing variable name for unset"); + return 0; + } +diff --git a/pppd/plugins/pppol2tp/openl2tp.c b/pppd/plugins/pppol2tp/openl2tp.c +index 1099575..7c4fe8b 100644 +--- a/pppd/plugins/pppol2tp/openl2tp.c ++++ b/pppd/plugins/pppol2tp/openl2tp.c +@@ -246,6 +246,9 @@ out: + (*old_pppol2tp_ip_updown_hook)(tunnel_id, session_id, up); + } + ++ if (user_name != NULL) ++ free(user_name); ++ + return; + } + +diff --git a/pppd/plugins/radius/avpair.c b/pppd/plugins/radius/avpair.c +index 716d23f..ec48eb8 100644 +--- a/pppd/plugins/radius/avpair.c ++++ b/pppd/plugins/radius/avpair.c +@@ -121,7 +121,8 @@ VALUE_PAIR *rc_avpair_new (int attrid, void *pval, int len, int vendorcode) + if ((vp = (VALUE_PAIR *) malloc (sizeof (VALUE_PAIR))) + != (VALUE_PAIR *) NULL) + { +- strncpy (vp->name, pda->name, sizeof (vp->name)); ++ strncpy (vp->name, pda->name, NAME_LENGTH); ++ vp->name[NAME_LENGTH] = 0; + vp->attribute = attrid; + vp->vendorcode = vendorcode; + vp->next = (VALUE_PAIR *) NULL; +diff --git a/pppd/plugins/radius/config.c b/pppd/plugins/radius/config.c +index a29e5e8..6e36d89 100644 +--- a/pppd/plugins/radius/config.c ++++ b/pppd/plugins/radius/config.c +@@ -153,6 +153,7 @@ static int set_option_auo(char *filename, int line, OPTION *option, char *p) + *iptr = AUTH_RADIUS_FST; + else { + error("%s: auth_order: unknown keyword: %s", filename, p); ++ free(iptr); + return (-1); + } + +@@ -165,6 +166,7 @@ static int set_option_auo(char *filename, int line, OPTION *option, char *p) + *iptr = (*iptr) | AUTH_RADIUS_SND; + else { + error("%s: auth_order: unknown or unexpected keyword: %s", filename, p); ++ free(iptr); + return (-1); + } + } +@@ -272,7 +274,7 @@ char *rc_conf_str(char *optname) + + if (option == NULL) + fatal("rc_conf_str: unkown config option requested: %s", optname); +- return (char *)option->val; ++ return (char *)option->val; + } + + int rc_conf_int(char *optname) +diff --git a/pppd/plugins/radius/radius.c b/pppd/plugins/radius/radius.c +index 4ba5f52..6f2a0bd 100644 +--- a/pppd/plugins/radius/radius.c ++++ b/pppd/plugins/radius/radius.c +@@ -898,7 +898,8 @@ radius_acct_start(void) + + rstate.start_time = time(NULL); + +- strncpy(rstate.session_id, rc_mksid(), sizeof(rstate.session_id)); ++ strncpy(rstate.session_id, rc_mksid(), MAXSESSIONID); ++ rstate.session_id[MAXSESSIONID] = 0; + + rc_avpair_add(&send, PW_ACCT_SESSION_ID, + rstate.session_id, 0, VENDOR_NONE); +diff --git a/pppd/plugins/radius/radiusclient.h b/pppd/plugins/radius/radiusclient.h +index 51b959a..cff0c26 100644 +--- a/pppd/plugins/radius/radiusclient.h ++++ b/pppd/plugins/radius/radiusclient.h +@@ -440,6 +440,7 @@ UINT4 rc_get_ipaddr __P((char *)); + int rc_good_ipaddr __P((char *)); + const char *rc_ip_hostname __P((UINT4)); + UINT4 rc_own_ipaddress __P((void)); ++UINT4 rc_own_bind_ipaddress __P((void)); + + + /* sendserver.c */ +diff --git a/pppd/plugins/radius/radrealms.c b/pppd/plugins/radius/radrealms.c +index 7a30370..cd006fd 100644 +--- a/pppd/plugins/radius/radrealms.c ++++ b/pppd/plugins/radius/radrealms.c +@@ -68,10 +68,12 @@ lookup_realm(char const *user, + + if ((fd = fopen(radrealms_config, "r")) == NULL) { + option_error("cannot open %s", radrealms_config); ++ free(auths); ++ free(accts); + return; +- } ++ } + info("Reading %s", radrealms_config); +- ++ + while ((fgets(buffer, sizeof(buffer), fd) != NULL)) { + line++; + +@@ -87,6 +89,8 @@ lookup_realm(char const *user, + fclose(fd); + option_error("%s: invalid line %d: %s", radrealms_config, + line, buffer); ++ free(auths); ++ free(accts); + return; + } + info("Parsing '%s' entry:", p); +@@ -101,6 +105,8 @@ lookup_realm(char const *user, + fclose(fd); + option_error("%s: realm name missing on line %d: %s", + radrealms_config, line, buffer); ++ free(auths); ++ free(accts); + return; + } + +@@ -111,6 +117,8 @@ lookup_realm(char const *user, + fclose(fd); + option_error("%s: server address missing on line %d: %s", + radrealms_config, line, buffer); ++ free(auths); ++ free(accts); + return; + } + s->name[s->max] = strdup(p); +@@ -119,6 +127,8 @@ lookup_realm(char const *user, + fclose(fd); + option_error("%s: server port missing on line %d: %s", + radrealms_config, line, buffer); ++ free(auths); ++ free(accts); + return; + } + s->port[s->max] = atoi(p); +diff --git a/pppd/plugins/rp-pppoe/Makefile.linux b/pppd/plugins/rp-pppoe/Makefile.linux +index 5e06b52..5f79284 100644 +--- a/pppd/plugins/rp-pppoe/Makefile.linux ++++ b/pppd/plugins/rp-pppoe/Makefile.linux +@@ -34,10 +34,10 @@ pppoe-discovery: pppoe-discovery.o debug.o common.o + $(CC) $(LDFLAGS) -o pppoe-discovery pppoe-discovery.o debug.o -ludev + + pppoe-discovery.o: pppoe-discovery.c +- $(CC) $(CFLAGS) -c -o pppoe-discovery.o pppoe-discovery.c ++ $(CC) $(CFLAGS) -I../../.. -c -o pppoe-discovery.o pppoe-discovery.c + + debug.o: debug.c +- $(CC) $(CFLAGS) -c -o debug.o debug.c ++ $(CC) $(CFLAGS) -I../../.. -c -o debug.o debug.c + + rp-pppoe.so: plugin.o discovery.o if.o common.o + $(CC) $(LDFLAGS) -o rp-pppoe.so -shared plugin.o discovery.o if.o common.o +diff --git a/pppd/plugins/rp-pppoe/if.c b/pppd/plugins/rp-pppoe/if.c +index 72aba41..50d5693 100644 +--- a/pppd/plugins/rp-pppoe/if.c ++++ b/pppd/plugins/rp-pppoe/if.c +@@ -133,7 +133,8 @@ openInterface(char const *ifname, UINT16_t type, unsigned char *hwaddr) + + /* Fill in hardware address */ + if (hwaddr) { +- strncpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)); ++ strncpy(ifr.ifr_name, ifname, IFNAMSIZ); ++ ifr.ifr_name[IFNAMSIZ - 1] = 0; + if (ioctl(fd, SIOCGIFHWADDR, &ifr) < 0) { + error("Can't get hardware address for %s: %m", ifname); + close(fd); +@@ -152,7 +153,8 @@ openInterface(char const *ifname, UINT16_t type, unsigned char *hwaddr) + } + + /* Sanity check on MTU */ +- strncpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)); ++ strncpy(ifr.ifr_name, ifname, IFNAMSIZ); ++ ifr.ifr_name[IFNAMSIZ - 1] = 0; + if (ioctl(fd, SIOCGIFMTU, &ifr) < 0) { + error("Can't get MTU for %s: %m", ifname); + } else if (ifr.ifr_mtu < ETH_DATA_LEN) { +@@ -166,7 +168,8 @@ openInterface(char const *ifname, UINT16_t type, unsigned char *hwaddr) + sa.sll_family = AF_PACKET; + sa.sll_protocol = htons(type); + +- strncpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)); ++ strncpy(ifr.ifr_name, ifname, IFNAMSIZ); ++ ifr.ifr_name[IFNAMSIZ - 1] = 0; + if (ioctl(fd, SIOCGIFINDEX, &ifr) < 0) { + error("Could not get interface index for %s: %m", ifname); + close(fd); +diff --git a/pppd/plugins/rp-pppoe/plugin.c b/pppd/plugins/rp-pppoe/plugin.c +index 24bdf8f..1856c6b 100644 +--- a/pppd/plugins/rp-pppoe/plugin.c ++++ b/pppd/plugins/rp-pppoe/plugin.c +@@ -153,7 +153,7 @@ PPPOEConnectDevice(void) + error("Can't get MTU for %s: %m", conn->ifName); + goto errout; + } +- strncpy(ifr.ifr_name, conn->ifName, sizeof(ifr.ifr_name)); ++ strlcpy(ifr.ifr_name, conn->ifName, sizeof(ifr.ifr_name)); + if (ioctl(s, SIOCGIFMTU, &ifr) < 0) { + error("Can't get MTU for %s: %m", conn->ifName); + close(s); +@@ -326,7 +326,7 @@ PPPoEDevnameHook(char *cmd, char **argv, int doit) + + /* Try getting interface index */ + if (r) { +- strncpy(ifr.ifr_name, cmd, sizeof(ifr.ifr_name)); ++ strlcpy(ifr.ifr_name, cmd, sizeof(ifr.ifr_name)); + if (ioctl(fd, SIOCGIFINDEX, &ifr) < 0) { + r = 0; + } else { +@@ -345,7 +345,7 @@ PPPoEDevnameHook(char *cmd, char **argv, int doit) + /* Close socket */ + close(fd); + if (r && doit) { +- strncpy(devnam, cmd, sizeof(devnam)); ++ strlcpy(devnam, cmd, sizeof(devnam)); + if (the_channel != &pppoe_channel) { + + the_channel = &pppoe_channel; +diff --git a/pppd/plugins/rp-pppoe/pppoe-discovery.c b/pppd/plugins/rp-pppoe/pppoe-discovery.c +index 2bd910f..502e17f 100644 +--- a/pppd/plugins/rp-pppoe/pppoe-discovery.c ++++ b/pppd/plugins/rp-pppoe/pppoe-discovery.c +@@ -177,7 +177,8 @@ openInterface(char const *ifname, UINT16_t type, unsigned char *hwaddr) + sa.sll_family = AF_PACKET; + sa.sll_protocol = htons(type); + +- strncpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)); ++ strncpy(ifr.ifr_name, ifname, IFNAMSIZ); ++ ifr.ifr_name[IFNAMSIZ - 1] = 0; + if (ioctl(fd, SIOCGIFINDEX, &ifr) < 0) { + fatalSys("ioctl(SIOCFIGINDEX): Could not get interface index"); + } +diff --git a/pppd/plugins/rp-pppoe/pppoe.h b/pppd/plugins/rp-pppoe/pppoe.h +index f77f5b7..6118e27 100644 +--- a/pppd/plugins/rp-pppoe/pppoe.h ++++ b/pppd/plugins/rp-pppoe/pppoe.h +@@ -24,6 +24,8 @@ + #include /* For FILE */ + #include /* For pid_t */ + ++#include "pppd/pppd.h" /* For error */ ++ + /* How do we access raw Ethernet devices? */ + #undef USE_LINUX_PACKET + #undef USE_BPF +diff --git a/pppd/plugins/winbind.c b/pppd/plugins/winbind.c +index bb05acd..4638f46 100644 +--- a/pppd/plugins/winbind.c ++++ b/pppd/plugins/winbind.c +@@ -432,6 +432,7 @@ unsigned int run_ntlm_auth(const char *username, + + /* parent */ + if (close(child_out[0]) == -1) { ++ close(child_in[1]); + notice("error closing pipe?!? for child OUT[0]"); + return NOT_AUTHENTICATED; + } +diff --git a/pppd/sys-linux.c b/pppd/sys-linux.c +index 9a1d8a6..ef92486 100644 +--- a/pppd/sys-linux.c ++++ b/pppd/sys-linux.c +@@ -2236,7 +2236,6 @@ int ppp_available(void) + } + } + +- close (s); + if (!ok) { + slprintf(route_buffer, sizeof(route_buffer), + "Sorry - PPP driver version %d.%d.%d is out of date\n", +@@ -2246,6 +2245,7 @@ int ppp_available(void) + } + } + } ++ close(s); + return ok; + } + +@@ -2722,7 +2722,10 @@ get_pty(master_fdp, slave_fdp, slave_name, uid) + warn("Couldn't unlock pty slave %s: %m", pty_name); + #endif + if ((sfd = open(pty_name, O_RDWR | O_NOCTTY | O_CLOEXEC)) < 0) ++ { + warn("Couldn't open pty slave %s: %m", pty_name); ++ close(mfd); ++ } + } + } + #endif /* TIOCGPTN */ +@@ -3011,6 +3014,7 @@ ether_to_eui64(eui64_t *p_eui64) + if (get_first_ethernet(ð_dev) < 0) + { + warn("no ethernet device present on the host"); ++ close(skfd); + return 0; + } + +diff --git a/pppstats/pppstats.c b/pppstats/pppstats.c +index 6367988..4aaa319 100644 +--- a/pppstats/pppstats.c ++++ b/pppstats/pppstats.c +@@ -150,7 +150,8 @@ get_ppp_stats(curp) + #define ifr_name ifr__name + #endif + +- strncpy(req.ifr_name, interface, sizeof(req.ifr_name)); ++ strncpy(req.ifr_name, interface, IFNAMSIZ); ++ req.ifr_name[IFNAMSIZ - 1] = 0; + if (ioctl(s, SIOCGPPPSTATS, &req) < 0) { + fprintf(stderr, "%s: ", progname); + if (errno == ENOTTY) +@@ -176,7 +177,8 @@ get_ppp_cstats(csp) + #define ifr_name ifr__name + #endif + +- strncpy(creq.ifr_name, interface, sizeof(creq.ifr_name)); ++ strncpy(creq.ifr_name, interface, IFNAMSIZ); ++ creq.ifr_name[IFNAMSIZ - 1] = 0; + if (ioctl(s, SIOCGPPPCSTATS, &creq) < 0) { + fprintf(stderr, "%s: ", progname); + if (errno == ENOTTY) { +@@ -526,7 +528,8 @@ main(argc, argv) + #undef ifr_name + #define ifr_name ifr_ifrn.ifrn_name + #endif +- strncpy(ifr.ifr_name, interface, sizeof(ifr.ifr_name)); ++ strncpy(ifr.ifr_name, interface, IFNAMSIZ); ++ ifr.ifr_name[IFNAMSIZ - 1] = 0; + if (ioctl(s, SIOCGIFFLAGS, (caddr_t)&ifr) < 0) { + fprintf(stderr, "%s: nonexistent interface '%s' specified\n", + progname, interface); diff --git a/ppp.spec b/ppp.spec index 2d2a229..c07c3d4 100644 --- a/ppp.spec +++ b/ppp.spec @@ -2,7 +2,7 @@ Name: ppp Version: 2.4.7 -Release: 28%{?dist} +Release: 29%{?dist} Summary: The Point-to-Point Protocol daemon License: BSD and LGPLv2+ and GPLv2+ and Public Domain URL: http://www.samba.org/ppp @@ -53,6 +53,7 @@ Patch0028: 0028-pppoe-include-netinet-in.h-before-linux-in.h.patch Patch0029: ppp-2.4.7-DES-openssl.patch # https://github.com/paulusmack/ppp/pull/95 Patch0030: ppp-2.4.7-honor-ldflags.patch +Patch0031: ppp-2.4.7-coverity-scan-fixes.patch BuildRequires: gcc BuildRequires: pam-devel, libpcap-devel, systemd, systemd-devel, glib2-devel @@ -183,6 +184,9 @@ install -p %{SOURCE11} %{buildroot}%{_sysconfdir}/sysconfig/network-scripts/ifdo %doc PLUGINS %changelog +* Mon Dec 3 2018 Jaroslav Škarvada - 2.4.7-29 +- Fixed some issues found by coverity scan + * Tue Nov 20 2018 Jaroslav Škarvada - 2.4.7-28 - Fixed network scripts related regression caused by release 26