Skip to content
Permalink
Browse files

MFC r350645: Correct ICMPv6/MLDv2 out-of-bounds memory access

Previously the ICMPv6 input path incorrectly handled cases where an
MLDv2 listener query packet was internally fragmented across multiple
mbufs.

admbugs:	921
Submitted by:	jtl
Reported by:	CJD of Apple
Approved by:	so
Security:	CVE-2019-5608
  • Loading branch information...
emaste committed Aug 6, 2019
1 parent be804d7 commit 6d7f541fdbb75dd9cc790d444ff37e07c5fdeb3e
Showing with 23 additions and 25 deletions.
  1. +23 −25 sys/netinet6/mld6.c
@@ -139,14 +139,15 @@ static int mld_v2_enqueue_group_record(struct mbufq *,
struct in6_multi *, const int, const int, const int,
const int);
static int mld_v2_input_query(struct ifnet *, const struct ip6_hdr *,
struct mbuf *, const int, const int);
struct mbuf *, struct mldv2_query *, const int, const int);
static int mld_v2_merge_state_changes(struct in6_multi *,
struct mbufq *);
static void mld_v2_process_group_timers(struct in6_multi_head *,
struct mbufq *, struct mbufq *,
struct in6_multi *, const int);
static int mld_v2_process_group_query(struct in6_multi *,
struct mld_ifsoftc *mli, int, struct mbuf *, const int);
struct mld_ifsoftc *mli, int, struct mbuf *,
struct mldv2_query *, const int);
static int sysctl_mld_gsr(SYSCTL_HANDLER_ARGS);
static int sysctl_mld_ifinfo(SYSCTL_HANDLER_ARGS);

@@ -803,16 +804,16 @@ mld_v1_update_group(struct in6_multi *inm, const int timer)
* Process a received MLDv2 general, group-specific or
* group-and-source-specific query.
*
* Assumes that the query header has been pulled up to sizeof(mldv2_query).
* Assumes that mld points to a struct mldv2_query which is stored in
* contiguous memory.
*
* Return 0 if successful, otherwise an appropriate error code is returned.
*/
static int
mld_v2_input_query(struct ifnet *ifp, const struct ip6_hdr *ip6,
struct mbuf *m, const int off, const int icmp6len)
struct mbuf *m, struct mldv2_query *mld, const int off, const int icmp6len)
{
struct mld_ifsoftc *mli;
struct mldv2_query *mld;
struct in6_multi *inm;
uint32_t maxdelay, nsrc, qqi;
int is_general_query;
@@ -844,8 +845,6 @@ mld_v2_input_query(struct ifnet *ifp, const struct ip6_hdr *ip6,

CTR2(KTR_MLD, "input v2 query on ifp %p(%s)", ifp, if_name(ifp));

mld = (struct mldv2_query *)(mtod(m, uint8_t *) + off);

maxdelay = ntohs(mld->mld_maxdelay); /* in 1/10ths of a second */
if (maxdelay >= 32768) {
maxdelay = (MLD_MRC_MANT(maxdelay) | 0x1000) <<
@@ -970,7 +969,7 @@ mld_v2_input_query(struct ifnet *ifp, const struct ip6_hdr *ip6,
* group-specific or group-and-source query.
*/
if (mli->mli_v2_timer == 0 || mli->mli_v2_timer >= timer)
mld_v2_process_group_query(inm, mli, timer, m, off);
mld_v2_process_group_query(inm, mli, timer, m, mld, off);

/* XXX Clear embedded scope ID as userland won't expect it. */
in6_clearscope(&mld->mld_addr);
@@ -991,17 +990,15 @@ mld_v2_input_query(struct ifnet *ifp, const struct ip6_hdr *ip6,
*/
static int
mld_v2_process_group_query(struct in6_multi *inm, struct mld_ifsoftc *mli,
int timer, struct mbuf *m0, const int off)
int timer, struct mbuf *m0, struct mldv2_query *mld, const int off)
{
struct mldv2_query *mld;
int retval;
uint16_t nsrc;

IN6_MULTI_LIST_LOCK_ASSERT();
MLD_LOCK_ASSERT();

retval = 0;
mld = (struct mldv2_query *)(mtod(m0, uint8_t *) + off);

switch (inm->in6m_state) {
case MLD_NOT_MEMBER:
@@ -1021,6 +1018,15 @@ mld_v2_process_group_query(struct in6_multi *inm, struct mld_ifsoftc *mli,

nsrc = ntohs(mld->mld_numsrc);

/* Length should be checked by calling function. */
KASSERT((m0->m_flags & M_PKTHDR) == 0 ||
m0->m_pkthdr.len >= off + sizeof(struct mldv2_query) +
nsrc * sizeof(struct in6_addr),
("mldv2 packet is too short: (%d bytes < %zd bytes, m=%p)",
m0->m_pkthdr.len, off + sizeof(struct mldv2_query) +
nsrc * sizeof(struct in6_addr), m0));


/*
* Deal with group-specific queries upfront.
* If any group query is already pending, purge any recorded
@@ -1062,28 +1068,20 @@ mld_v2_process_group_query(struct in6_multi *inm, struct mld_ifsoftc *mli,
* report for those sources.
*/
if (inm->in6m_nsrc > 0) {
struct mbuf *m;
uint8_t *sp;
struct in6_addr srcaddr;
int i, nrecorded;
int soff;

m = m0;
soff = off + sizeof(struct mldv2_query);
nrecorded = 0;
for (i = 0; i < nsrc; i++) {
sp = mtod(m, uint8_t *) + soff;
retval = in6m_record_source(inm,
(const struct in6_addr *)sp);
m_copydata(m0, soff, sizeof(struct in6_addr),
(caddr_t)&srcaddr);
retval = in6m_record_source(inm, &srcaddr);
if (retval < 0)
break;
nrecorded += retval;
soff += sizeof(struct in6_addr);
if (soff >= m->m_len) {
soff = soff - m->m_len;
m = m->m_next;
if (m == NULL)
break;
}
}
if (nrecorded > 0) {
CTR1(KTR_MLD,
@@ -1292,8 +1290,8 @@ mld_input(struct mbuf *m, int off, int icmp6len)
if (mld_v1_input_query(ifp, ip6, mld) != 0)
return (0);
} else if (icmp6len >= sizeof(struct mldv2_query)) {
if (mld_v2_input_query(ifp, ip6, m, off,
icmp6len) != 0)
if (mld_v2_input_query(ifp, ip6, m,
(struct mldv2_query *)mld, off, icmp6len) != 0)
return (0);
}
break;

0 comments on commit 6d7f541

Please sign in to comment.
You can’t perform that action at this time.