Merge pull request #2623 from pacovn/PVS-Studio_memcpy_source_underflow

ldpd: buffer underflow, thread safety (PVS-Studio)
This commit is contained in:
Quentin Young 2018-07-05 17:41:25 -04:00 committed by GitHub
commit b1b5a009c6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 46 additions and 51 deletions

View File

@ -691,7 +691,8 @@ void embedscope(struct sockaddr_in6 *);
void recoverscope(struct sockaddr_in6 *); void recoverscope(struct sockaddr_in6 *);
void addscope(struct sockaddr_in6 *, uint32_t); void addscope(struct sockaddr_in6 *, uint32_t);
void clearscope(struct in6_addr *); void clearscope(struct in6_addr *);
struct sockaddr *addr2sa(int af, union ldpd_addr *, uint16_t); void addr2sa(int af, const union ldpd_addr *, uint16_t,
union sockunion *su);
void sa2addr(struct sockaddr *, int *, union ldpd_addr *, void sa2addr(struct sockaddr *, int *, union ldpd_addr *,
in_port_t *); in_port_t *);
socklen_t sockaddr_len(struct sockaddr *); socklen_t sockaddr_len(struct sockaddr *);

View File

@ -584,8 +584,8 @@ nbr_connect_cb(struct thread *thread)
int int
nbr_establish_connection(struct nbr *nbr) nbr_establish_connection(struct nbr *nbr)
{ {
struct sockaddr_storage local_sa; union sockunion local_su;
struct sockaddr_storage remote_sa; union sockunion remote_su;
struct adj *adj; struct adj *adj;
struct nbr_params *nbrp; struct nbr_params *nbrp;
#ifdef __OpenBSD__ #ifdef __OpenBSD__
@ -619,16 +619,14 @@ nbr_establish_connection(struct nbr *nbr)
#endif #endif
} }
memcpy(&local_sa, addr2sa(nbr->af, &nbr->laddr, 0), sizeof(local_sa)); addr2sa(nbr->af, &nbr->laddr, 0, &local_su);
memcpy(&remote_sa, addr2sa(nbr->af, &nbr->raddr, LDP_PORT), addr2sa(nbr->af, &nbr->raddr, LDP_PORT, &remote_su);
sizeof(local_sa));
if (nbr->af == AF_INET6 && nbr->raddr_scope) if (nbr->af == AF_INET6 && nbr->raddr_scope)
addscope((struct sockaddr_in6 *)&remote_sa, nbr->raddr_scope); addscope(&remote_su.sin6, nbr->raddr_scope);
if (bind(nbr->fd, (struct sockaddr *)&local_sa, if (bind(nbr->fd, &local_su.sa, sockaddr_len(&local_su.sa)) == -1) {
sockaddr_len((struct sockaddr *)&local_sa)) == -1) {
log_warn("%s: error while binding socket to %s", __func__, log_warn("%s: error while binding socket to %s", __func__,
log_sockaddr((struct sockaddr *)&local_sa)); log_sockaddr(&local_su.sa));
close(nbr->fd); close(nbr->fd);
return (-1); return (-1);
} }
@ -646,15 +644,15 @@ nbr_establish_connection(struct nbr *nbr)
send_hello(adj->source.type, adj->source.link.ia, send_hello(adj->source.type, adj->source.link.ia,
adj->source.target); adj->source.target);
if (connect(nbr->fd, (struct sockaddr *)&remote_sa, if (connect(nbr->fd, &remote_su.sa, sockaddr_len(&remote_su.sa))
sockaddr_len((struct sockaddr *)&remote_sa)) == -1) { == -1) {
if (errno == EINPROGRESS) { if (errno == EINPROGRESS) {
thread_add_write(master, nbr_connect_cb, nbr, nbr->fd, thread_add_write(master, nbr_connect_cb, nbr, nbr->fd,
&nbr->ev_connect); &nbr->ev_connect);
return (0); return (0);
} }
log_warn("%s: error while connecting to %s", __func__, log_warn("%s: error while connecting to %s", __func__,
log_sockaddr((struct sockaddr *)&remote_sa)); log_sockaddr(&remote_su.sa));
close(nbr->fd); close(nbr->fd);
return (-1); return (-1);
} }

View File

@ -70,7 +70,7 @@ int
send_packet(int fd, int af, union ldpd_addr *dst, struct iface_af *ia, send_packet(int fd, int af, union ldpd_addr *dst, struct iface_af *ia,
void *pkt, size_t len) void *pkt, size_t len)
{ {
struct sockaddr *sa; union sockunion su;
switch (af) { switch (af) {
case AF_INET: case AF_INET:
@ -97,10 +97,10 @@ send_packet(int fd, int af, union ldpd_addr *dst, struct iface_af *ia,
fatalx("send_packet: unknown af"); fatalx("send_packet: unknown af");
} }
sa = addr2sa(af, dst, LDP_PORT); addr2sa(af, dst, LDP_PORT, &su);
if (sendto(fd, pkt, len, 0, sa, sockaddr_len(sa)) == -1) { if (sendto(fd, pkt, len, 0, &su.sa, sockaddr_len(&su.sa)) == -1) {
log_warn("%s: error sending packet to %s", __func__, log_warn("%s: error sending packet to %s", __func__,
log_sockaddr(sa)); log_sockaddr(&su.sa));
return (-1); return (-1);
} }

View File

@ -64,17 +64,17 @@ pfkey_send(int sd, uint8_t satype, uint8_t mtype, uint8_t dir,
ssize_t n; ssize_t n;
int len = 0; int len = 0;
int iov_cnt; int iov_cnt;
struct sockaddr_storage ssrc, sdst, smask, dmask; struct sockaddr_storage smask, dmask;
struct sockaddr *saptr; union sockunion su_src, su_dst;
if (!pid) if (!pid)
pid = getpid(); pid = getpid();
/* we need clean sockaddr... no ports set */ /* we need clean sockaddr... no ports set */
memset(&ssrc, 0, sizeof(ssrc));
memset(&smask, 0, sizeof(smask)); memset(&smask, 0, sizeof(smask));
if ((saptr = addr2sa(af, src, 0)))
memcpy(&ssrc, saptr, sizeof(ssrc)); addr2sa(af, src, 0, &su_src);
switch (af) { switch (af) {
case AF_INET: case AF_INET:
memset(&((struct sockaddr_in *)&smask)->sin_addr, 0xff, 32/8); memset(&((struct sockaddr_in *)&smask)->sin_addr, 0xff, 32/8);
@ -86,13 +86,13 @@ pfkey_send(int sd, uint8_t satype, uint8_t mtype, uint8_t dir,
default: default:
return (-1); return (-1);
} }
smask.ss_family = ssrc.ss_family; smask.ss_family = su_src.sa.sa_family;
smask.ss_len = ssrc.ss_len; smask.ss_len = sockaddr_len(&su_src.sa);
memset(&sdst, 0, sizeof(sdst));
memset(&dmask, 0, sizeof(dmask)); memset(&dmask, 0, sizeof(dmask));
if ((saptr = addr2sa(af, dst, 0)))
memcpy(&sdst, saptr, sizeof(sdst)); addr2sa(af, dst, 0, &su_dst);
switch (af) { switch (af) {
case AF_INET: case AF_INET:
memset(&((struct sockaddr_in *)&dmask)->sin_addr, 0xff, 32/8); memset(&((struct sockaddr_in *)&dmask)->sin_addr, 0xff, 32/8);
@ -104,8 +104,8 @@ pfkey_send(int sd, uint8_t satype, uint8_t mtype, uint8_t dir,
default: default:
return (-1); return (-1);
} }
dmask.ss_family = sdst.ss_family; dmask.ss_family = su_dst.sa.sa_family;
dmask.ss_len = sdst.ss_len; dmask.ss_len = sockaddr_len(&su_dst.sa);
memset(&smsg, 0, sizeof(smsg)); memset(&smsg, 0, sizeof(smsg));
smsg.sadb_msg_version = PF_KEY_V2; smsg.sadb_msg_version = PF_KEY_V2;
@ -138,11 +138,13 @@ pfkey_send(int sd, uint8_t satype, uint8_t mtype, uint8_t dir,
memset(&sa_src, 0, sizeof(sa_src)); memset(&sa_src, 0, sizeof(sa_src));
sa_src.sadb_address_exttype = SADB_EXT_ADDRESS_SRC; sa_src.sadb_address_exttype = SADB_EXT_ADDRESS_SRC;
sa_src.sadb_address_len = (sizeof(sa_src) + ROUNDUP(ssrc.ss_len)) / 8; sa_src.sadb_address_len =
(sizeof(sa_src) + ROUNDUP(sockaddr_len(&su_src.sa))) / 8;
memset(&sa_dst, 0, sizeof(sa_dst)); memset(&sa_dst, 0, sizeof(sa_dst));
sa_dst.sadb_address_exttype = SADB_EXT_ADDRESS_DST; sa_dst.sadb_address_exttype = SADB_EXT_ADDRESS_DST;
sa_dst.sadb_address_len = (sizeof(sa_dst) + ROUNDUP(sdst.ss_len)) / 8; sa_dst.sadb_address_len =
(sizeof(sa_dst) + ROUNDUP(sockaddr_len(&su_dst.sa))) / 8;
sa.sadb_sa_auth = aalg; sa.sadb_sa_auth = aalg;
sa.sadb_sa_encrypt = SADB_X_EALG_AES; /* XXX */ sa.sadb_sa_encrypt = SADB_X_EALG_AES; /* XXX */
@ -195,8 +197,8 @@ pfkey_send(int sd, uint8_t satype, uint8_t mtype, uint8_t dir,
iov[iov_cnt].iov_base = &sa_dst; iov[iov_cnt].iov_base = &sa_dst;
iov[iov_cnt].iov_len = sizeof(sa_dst); iov[iov_cnt].iov_len = sizeof(sa_dst);
iov_cnt++; iov_cnt++;
iov[iov_cnt].iov_base = &sdst; iov[iov_cnt].iov_base = &su_dst;
iov[iov_cnt].iov_len = ROUNDUP(sdst.ss_len); iov[iov_cnt].iov_len = ROUNDUP(sockaddr_len(&su_dst.sa));
smsg.sadb_msg_len += sa_dst.sadb_address_len; smsg.sadb_msg_len += sa_dst.sadb_address_len;
iov_cnt++; iov_cnt++;
@ -204,8 +206,8 @@ pfkey_send(int sd, uint8_t satype, uint8_t mtype, uint8_t dir,
iov[iov_cnt].iov_base = &sa_src; iov[iov_cnt].iov_base = &sa_src;
iov[iov_cnt].iov_len = sizeof(sa_src); iov[iov_cnt].iov_len = sizeof(sa_src);
iov_cnt++; iov_cnt++;
iov[iov_cnt].iov_base = &ssrc; iov[iov_cnt].iov_base = &su_src;
iov[iov_cnt].iov_len = ROUNDUP(ssrc.ss_len); iov[iov_cnt].iov_len = ROUNDUP(sockaddr_len(&su_src.sa));
smsg.sadb_msg_len += sa_src.sadb_address_len; smsg.sadb_msg_len += sa_src.sadb_address_len;
iov_cnt++; iov_cnt++;

View File

@ -37,7 +37,7 @@ ldp_create_socket(int af, enum socket_type type)
{ {
int fd, domain, proto; int fd, domain, proto;
union ldpd_addr addr; union ldpd_addr addr;
struct sockaddr_storage local_sa; union sockunion local_su;
#ifdef __OpenBSD__ #ifdef __OpenBSD__
int opt; int opt;
#endif #endif
@ -70,14 +70,12 @@ ldp_create_socket(int af, enum socket_type type)
case LDP_SOCKET_DISC: case LDP_SOCKET_DISC:
/* listen on all addresses */ /* listen on all addresses */
memset(&addr, 0, sizeof(addr)); memset(&addr, 0, sizeof(addr));
memcpy(&local_sa, addr2sa(af, &addr, LDP_PORT), addr2sa(af, &addr, LDP_PORT, &local_su);
sizeof(local_sa));
break; break;
case LDP_SOCKET_EDISC: case LDP_SOCKET_EDISC:
case LDP_SOCKET_SESSION: case LDP_SOCKET_SESSION:
addr = (ldp_af_conf_get(ldpd_conf, af))->trans_addr; addr = (ldp_af_conf_get(ldpd_conf, af))->trans_addr;
memcpy(&local_sa, addr2sa(af, &addr, LDP_PORT), addr2sa(af, &addr, LDP_PORT, &local_su);
sizeof(local_sa));
/* ignore any possible error */ /* ignore any possible error */
sock_set_bindany(fd, 1); sock_set_bindany(fd, 1);
break; break;
@ -90,8 +88,7 @@ ldp_create_socket(int af, enum socket_type type)
close(fd); close(fd);
return (-1); return (-1);
} }
if (bind(fd, (struct sockaddr *)&local_sa, if (bind(fd, &local_su.sa, sockaddr_len(&local_su.sa)) == -1) {
sockaddr_len((struct sockaddr *)&local_sa)) == -1) {
save_errno = errno; save_errno = errno;
if (ldpd_privs.change(ZPRIVS_LOWER)) if (ldpd_privs.change(ZPRIVS_LOWER))
log_warn("%s: could not lower privs", __func__); log_warn("%s: could not lower privs", __func__);
@ -307,7 +304,7 @@ sock_set_md5sig(int fd, int af, union ldpd_addr *addr, const char *password)
if (fd == -1) if (fd == -1)
return (0); return (0);
#if HAVE_DECL_TCP_MD5SIG #if HAVE_DECL_TCP_MD5SIG
memcpy(&su, addr2sa(af, addr, 0), sizeof(su)); addr2sa(af, addr, 0, &su);
if (ldpe_privs.change(ZPRIVS_RAISE)) { if (ldpe_privs.change(ZPRIVS_RAISE)) {
log_warn("%s: could not raise privs", __func__); log_warn("%s: could not raise privs", __func__);

View File

@ -305,14 +305,13 @@ clearscope(struct in6_addr *in6)
} }
} }
struct sockaddr * void
addr2sa(int af, union ldpd_addr *addr, uint16_t port) addr2sa(int af, const union ldpd_addr *addr, uint16_t port, union sockunion *su)
{ {
static struct sockaddr_storage ss; struct sockaddr_in *sa_in = &su->sin;
struct sockaddr_in *sa_in = (struct sockaddr_in *)&ss; struct sockaddr_in6 *sa_in6 = &su->sin6;
struct sockaddr_in6 *sa_in6 = (struct sockaddr_in6 *)&ss;
memset(&ss, 0, sizeof(ss)); memset(su, 0, sizeof(*su));
switch (af) { switch (af) {
case AF_INET: case AF_INET:
sa_in->sin_family = AF_INET; sa_in->sin_family = AF_INET;
@ -333,8 +332,6 @@ addr2sa(int af, union ldpd_addr *addr, uint16_t port)
default: default:
fatalx("addr2sa: unknown af"); fatalx("addr2sa: unknown af");
} }
return ((struct sockaddr *)&ss);
} }
void void