Date: Tue, 7 Apr 2015 20:20:47 +0000 (UTC) From: Xin LI <delphij@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org Subject: svn commit: r281231 - in stable: 8/contrib/ntp/ntpd 8/sys/netinet 8/sys/netinet6 9/contrib/ntp/ntpd 9/sys/netinet 9/sys/netinet6 Message-ID: <201504072020.t37KKl5Q032136@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: delphij Date: Tue Apr 7 20:20:44 2015 New Revision: 281231 URL: https://svnweb.freebsd.org/changeset/base/281231 Log: Improve patch for SA-15:04.igmp to solve a potential buffer overflow. Fix multiple vulnerabilities of ntp. [SA-15:07] Fix Denial of Service with IPv6 Router Advertisements. [SA-15:09] Modified: stable/8/contrib/ntp/ntpd/ntp_crypto.c stable/8/contrib/ntp/ntpd/ntp_proto.c stable/8/sys/netinet/igmp.c stable/8/sys/netinet6/nd6_rtr.c Changes in other areas also in this revision: Modified: stable/9/contrib/ntp/ntpd/ntp_crypto.c stable/9/contrib/ntp/ntpd/ntp_proto.c stable/9/sys/netinet/igmp.c stable/9/sys/netinet6/nd6_rtr.c Modified: stable/8/contrib/ntp/ntpd/ntp_crypto.c ============================================================================== --- stable/8/contrib/ntp/ntpd/ntp_crypto.c Tue Apr 7 20:20:24 2015 (r281230) +++ stable/8/contrib/ntp/ntpd/ntp_crypto.c Tue Apr 7 20:20:44 2015 (r281231) @@ -93,6 +93,7 @@ #define TAI_1972 10 /* initial TAI offset (s) */ #define MAX_LEAP 100 /* max UTC leapseconds (s) */ #define VALUE_LEN (6 * 4) /* min response field length */ +#define MAX_VALLEN (65535 - VALUE_LEN) #define YEAR (60 * 60 * 24 * 365) /* seconds in year */ /* @@ -137,8 +138,8 @@ static u_int ident_scheme = 0; /* server */ static int crypto_verify P((struct exten *, struct value *, struct peer *)); -static int crypto_encrypt P((struct exten *, struct value *, - keyid_t *)); +static int crypto_encrypt P((const u_char *, u_int, keyid_t *, + struct value *)); static int crypto_alice P((struct peer *, struct value *)); static int crypto_alice2 P((struct peer *, struct value *)); static int crypto_alice3 P((struct peer *, struct value *)); @@ -446,6 +447,12 @@ crypto_recv( tstamp = ntohl(ep->tstamp); fstamp = ntohl(ep->fstamp); vallen = ntohl(ep->vallen); + /* + * Bug 2761: I hope this isn't too early... + */ + if ( vallen == 0 + || len - VALUE_LEN < vallen) + return XEVNT_LEN; } switch (code) { @@ -488,7 +495,7 @@ crypto_recv( break; if (vallen == 0 || vallen > MAXHOSTNAME || - len < VALUE_LEN + vallen) { + len - VALUE_LEN < vallen) { rval = XEVNT_LEN; break; } @@ -1250,7 +1257,8 @@ crypto_xmit( vallen = ntohl(ep->vallen); if (vallen == 8) { strcpy(certname, sys_hostname); - } else if (vallen == 0 || vallen > MAXHOSTNAME) { + } else if (vallen == 0 || vallen > MAXHOSTNAME || + len - VALUE_LEN < vallen) { rval = XEVNT_LEN; break; @@ -1407,7 +1415,10 @@ crypto_xmit( * anything goes wrong. */ case CRYPTO_COOK | CRYPTO_RESP: - if ((opcode & 0xffff) < VALUE_LEN) { + vallen = ntohl(ep->vallen); /* Must be <64k */ + if ( vallen == 0 + || (vallen >= MAX_VALLEN) + || (opcode & 0x0000ffff) < VALUE_LEN + vallen) { rval = XEVNT_LEN; break; } @@ -1420,10 +1431,11 @@ crypto_xmit( } tcookie = peer->pcookie; } - if ((rval = crypto_encrypt(ep, &vtemp, &tcookie)) == - XEVNT_OK) + if ((rval = crypto_encrypt((const u_char *)ep->pkt, vallen, &tcookie, &vtemp)) + == XEVNT_OK) { len += crypto_send(fp, &vtemp); - value_free(&vtemp); + value_free(&vtemp); + } break; /* @@ -1558,10 +1570,15 @@ crypto_verify( * are rounded up to the next word. */ vallen = ntohl(ep->vallen); + if ( vallen == 0 + || vallen > MAX_VALLEN) + return (XEVNT_LEN); i = (vallen + 3) / 4; siglen = ntohl(ep->pkt[i++]); - if (len < VALUE_LEN + ((vallen + 3) / 4) * 4 + ((siglen + 3) / - 4) * 4) + if ( siglen > MAX_VALLEN + || len - VALUE_LEN < ((vallen + 3) / 4) * 4 + || len - VALUE_LEN - ((vallen + 3) / 4) * 4 + < ((siglen + 3) / 4) * 4) return (XEVNT_LEN); /* @@ -1627,6 +1644,7 @@ crypto_verify( * avoid doing the sign exchange. */ EVP_VerifyInit(&ctx, peer->digest); + /* XXX: the "+ 12" needs to be at least documented... */ EVP_VerifyUpdate(&ctx, (u_char *)&ep->tstamp, vallen + 12); if (EVP_VerifyFinal(&ctx, (u_char *)&ep->pkt[i], siglen, pkey) <= 0) return (XEVNT_SIG); @@ -1641,10 +1659,10 @@ crypto_verify( /* - * crypto_encrypt - construct encrypted cookie and signature from - * extension field and cookie + * crypto_encrypt - construct vp (encrypted cookie and signature) from + * the public key and cookie. * - * Returns + * Returns: * XEVNT_OK success * XEVNT_PUB bad or missing public key * XEVNT_CKY bad or missing cookie @@ -1652,24 +1670,21 @@ crypto_verify( */ static int crypto_encrypt( - struct exten *ep, /* extension pointer */ - struct value *vp, /* value pointer */ - keyid_t *cookie /* server cookie */ + const u_char *ptr, /* Public Key */ + u_int vallen, /* Length of Public Key */ + keyid_t *cookie, /* server cookie */ + struct value *vp /* value pointer */ ) { EVP_PKEY *pkey; /* public key */ EVP_MD_CTX ctx; /* signature context */ tstamp_t tstamp; /* NTP timestamp */ u_int32 temp32; - u_int len; - u_char *ptr; /* * Extract the public key from the request. */ - len = ntohl(ep->vallen); - ptr = (u_char *)ep->pkt; - pkey = d2i_PublicKey(EVP_PKEY_RSA, NULL, &ptr, len); + pkey = d2i_PublicKey(EVP_PKEY_RSA, NULL, &ptr, vallen); if (pkey == NULL) { msyslog(LOG_ERR, "crypto_encrypt %s\n", ERR_error_string(ERR_get_error(), NULL)); @@ -1683,9 +1698,9 @@ crypto_encrypt( memset(vp, 0, sizeof(struct value)); vp->tstamp = htonl(tstamp); vp->fstamp = hostval.tstamp; - len = EVP_PKEY_size(pkey); - vp->vallen = htonl(len); - vp->ptr = emalloc(len); + vallen = EVP_PKEY_size(pkey); + vp->vallen = htonl(vallen); + vp->ptr = emalloc(vallen); temp32 = htonl(*cookie); if (!RSA_public_encrypt(4, (u_char *)&temp32, vp->ptr, pkey->pkey.rsa, RSA_PKCS1_OAEP_PADDING)) { @@ -1705,9 +1720,9 @@ crypto_encrypt( vp->sig = emalloc(sign_siglen); EVP_SignInit(&ctx, sign_digest); EVP_SignUpdate(&ctx, (u_char *)&vp->tstamp, 12); - EVP_SignUpdate(&ctx, vp->ptr, len); - if (EVP_SignFinal(&ctx, vp->sig, &len, sign_pkey)) - vp->siglen = htonl(len); + EVP_SignUpdate(&ctx, vp->ptr, vallen); + if (EVP_SignFinal(&ctx, vp->sig, &vallen, sign_pkey)) + vp->siglen = htonl(sign_siglen); return (XEVNT_OK); } @@ -1794,6 +1809,9 @@ crypto_ident( * call in the protocol module. * * Returns extension field pointer (no errors). + * + * XXX: opcode and len should really be 32-bit quantities and + * we should make sure that str is not too big. */ struct exten * crypto_args( @@ -1805,11 +1823,14 @@ crypto_args( tstamp_t tstamp; /* NTP timestamp */ struct exten *ep; /* extension field pointer */ u_int len; /* extension field length */ + size_t slen; tstamp = crypto_time(); len = sizeof(struct exten); - if (str != NULL) - len += strlen(str); + if (str != NULL) { + slen = strlen(str); + len += slen; + } ep = emalloc(len); memset(ep, 0, len); if (opcode == 0) @@ -1829,8 +1850,8 @@ crypto_args( ep->fstamp = hostval.tstamp; ep->vallen = 0; if (str != NULL) { - ep->vallen = htonl(strlen(str)); - memcpy((char *)ep->pkt, str, strlen(str)); + ep->vallen = htonl(slen); + memcpy((char *)ep->pkt, str, slen); } else { ep->pkt[0] = peer->associd; } @@ -1844,6 +1865,8 @@ crypto_args( * Returns extension field length. Note: it is not polite to send a * nonempty signature with zero timestamp or a nonzero timestamp with * empty signature, but these rules are not enforced here. + * + * XXX This code won't work on a box with 16-bit ints. */ u_int crypto_send( @@ -2212,7 +2235,8 @@ crypto_bob( tstamp_t tstamp; /* NTP timestamp */ BIGNUM *bn, *bk, *r; u_char *ptr; - u_int len; + u_int len; /* extension field length */ + u_int vallen = 0; /* value length */ /* * If the IFF parameters are not valid, something awful @@ -2227,8 +2251,11 @@ crypto_bob( /* * Extract r from the challenge. */ - len = ntohl(ep->vallen); - if ((r = BN_bin2bn((u_char *)ep->pkt, len, NULL)) == NULL) { + vallen = ntohl(ep->vallen); + len = ntohl(ep->opcode) & 0x0000ffff; + if (vallen == 0 || len < VALUE_LEN || len - VALUE_LEN < vallen) + return XEVNT_LEN; + if ((r = BN_bin2bn((u_char *)ep->pkt, vallen, NULL)) == NULL) { msyslog(LOG_ERR, "crypto_bob %s\n", ERR_error_string(ERR_get_error(), NULL)); return (XEVNT_ERR); @@ -2240,7 +2267,7 @@ crypto_bob( */ bctx = BN_CTX_new(); bk = BN_new(); bn = BN_new(); sdsa = DSA_SIG_new(); - BN_rand(bk, len * 8, -1, 1); /* k */ + BN_rand(bk, vallen * 8, -1, 1); /* k */ BN_mod_mul(bn, dsa->priv_key, r, dsa->q, bctx); /* b r mod q */ BN_add(bn, bn, bk); BN_mod(bn, bn, dsa->q, bctx); /* k + b r mod q */ @@ -2254,19 +2281,25 @@ crypto_bob( /* * Encode the values in ASN.1 and sign. */ - tstamp = crypto_time(); - memset(vp, 0, sizeof(struct value)); - vp->tstamp = htonl(tstamp); - vp->fstamp = htonl(if_fstamp); - len = i2d_DSA_SIG(sdsa, NULL); - if (len <= 0) { + vallen = i2d_DSA_SIG(sdsa, NULL); + if (vallen == 0) { msyslog(LOG_ERR, "crypto_bob %s\n", ERR_error_string(ERR_get_error(), NULL)); DSA_SIG_free(sdsa); return (XEVNT_ERR); } - vp->vallen = htonl(len); - ptr = emalloc(len); + if (vallen > MAX_VALLEN) { + msyslog(LOG_ERR, "crypto_bob: signature is too big: %d", + vallen); + DSA_SIG_free(sdsa); + return (XEVNT_LEN); + } + memset(vp, 0, sizeof(struct value)); + tstamp = crypto_time(); + vp->tstamp = htonl(tstamp); + vp->fstamp = htonl(if_fstamp); + vp->vallen = htonl(vallen); + ptr = emalloc(vallen); vp->ptr = ptr; i2d_DSA_SIG(sdsa, &ptr); DSA_SIG_free(sdsa); @@ -2277,11 +2310,12 @@ crypto_bob( if (tstamp < cinfo->first || tstamp > cinfo->last) return (XEVNT_PER); + /* XXX: more validation to make sure the sign fits... */ vp->sig = emalloc(sign_siglen); EVP_SignInit(&ctx, sign_digest); EVP_SignUpdate(&ctx, (u_char *)&vp->tstamp, 12); - EVP_SignUpdate(&ctx, vp->ptr, len); - if (EVP_SignFinal(&ctx, vp->sig, &len, sign_pkey)) + EVP_SignUpdate(&ctx, vp->ptr, vallen); + if (EVP_SignFinal(&ctx, vp->sig, &vallen, sign_pkey)) vp->siglen = htonl(len); return (XEVNT_OK); } Modified: stable/8/contrib/ntp/ntpd/ntp_proto.c ============================================================================== --- stable/8/contrib/ntp/ntpd/ntp_proto.c Tue Apr 7 20:20:24 2015 (r281230) +++ stable/8/contrib/ntp/ntpd/ntp_proto.c Tue Apr 7 20:20:44 2015 (r281231) @@ -459,7 +459,7 @@ receive( while (has_mac > 0) { int temp; - if (has_mac % 4 != 0 || has_mac < 0) { + if (has_mac % 4 != 0 || has_mac < MIN_MAC_LEN) { sys_badlength++; return; /* bad MAC length */ } @@ -483,6 +483,13 @@ receive( return; /* bad MAC length */ } } + /* + * If has_mac is < 0 we had a malformed packet. + */ + if (has_mac < 0) { + sys_badlength++; + return; /* bad length */ + } #ifdef OPENSSL pkeyid = tkeyid = 0; #endif /* OPENSSL */ @@ -942,12 +949,9 @@ receive( } /* - * Update the origin and destination timestamps. If - * unsynchronized or bogus abandon ship. If the crypto machine + * If unsynchronized or bogus abandon ship. If the crypto machine * breaks, light the crypto bit and plaint the log. */ - peer->org = p_xmt; - peer->rec = rbufp->recv_time; if (peer->flash & PKT_TEST_MASK) { #ifdef OPENSSL if (crypto_flags && (peer->flags & FLAG_SKEY)) { @@ -978,10 +982,11 @@ receive( * versions. If symmetric modes, return a crypto-NAK. The peer * should restart the protocol. */ - } else if (!AUTH(peer->keyid || (restrict_mask & RES_DONTTRUST), - is_authentic)) { + } else if (!AUTH(peer->keyid || has_mac || + (restrict_mask & RES_DONTTRUST), is_authentic)) { peer->flash |= TEST5; - if (hismode == MODE_ACTIVE || hismode == MODE_PASSIVE) + if (has_mac && + (hismode == MODE_ACTIVE || hismode == MODE_PASSIVE)) fast_xmit(rbufp, MODE_ACTIVE, 0, restrict_mask); return; /* bad auth */ } @@ -989,7 +994,12 @@ receive( /* * That was hard and I am sweaty, but the packet is squeaky * clean. Get on with real work. + * + * Update the origin and destination timestamps. */ + peer->org = p_xmt; + peer->rec = rbufp->recv_time; + peer->received++; peer->timereceived = current_time; if (is_authentic == AUTH_OK) Modified: stable/8/sys/netinet/igmp.c ============================================================================== --- stable/8/sys/netinet/igmp.c Tue Apr 7 20:20:24 2015 (r281230) +++ stable/8/sys/netinet/igmp.c Tue Apr 7 20:20:44 2015 (r281231) @@ -1533,7 +1533,6 @@ igmp_input(struct mbuf *m, int off) struct igmpv3 *igmpv3; uint16_t igmpv3len; uint16_t nsrc; - int srclen; IGMPSTAT_INC(igps_rcv_v3_queries); igmpv3 = (struct igmpv3 *)igmp; @@ -1541,8 +1540,8 @@ igmp_input(struct mbuf *m, int off) * Validate length based on source count. */ nsrc = ntohs(igmpv3->igmp_numsrc); - srclen = sizeof(struct in_addr) * nsrc; - if (nsrc * sizeof(in_addr_t) > srclen) { + if (nsrc * sizeof(in_addr_t) > + UINT16_MAX - iphlen - IGMP_V3_QUERY_MINLEN) { IGMPSTAT_INC(igps_rcv_tooshort); return; } @@ -1551,7 +1550,7 @@ igmp_input(struct mbuf *m, int off) * this scope. */ igmpv3len = iphlen + IGMP_V3_QUERY_MINLEN + - srclen; + sizeof(struct in_addr) * nsrc; if ((m->m_flags & M_EXT || m->m_len < igmpv3len) && (m = m_pullup(m, igmpv3len)) == NULL) { Modified: stable/8/sys/netinet6/nd6_rtr.c ============================================================================== --- stable/8/sys/netinet6/nd6_rtr.c Tue Apr 7 20:20:24 2015 (r281230) +++ stable/8/sys/netinet6/nd6_rtr.c Tue Apr 7 20:20:44 2015 (r281231) @@ -286,8 +286,16 @@ nd6_ra_input(struct mbuf *m, int off, in } if (nd_ra->nd_ra_retransmit) ndi->retrans = ntohl(nd_ra->nd_ra_retransmit); - if (nd_ra->nd_ra_curhoplimit) - ndi->chlim = nd_ra->nd_ra_curhoplimit; + if (nd_ra->nd_ra_curhoplimit) { + if (ndi->chlim < nd_ra->nd_ra_curhoplimit) + ndi->chlim = nd_ra->nd_ra_curhoplimit; + else if (ndi->chlim != nd_ra->nd_ra_curhoplimit) { + log(LOG_ERR, "RA with a lower CurHopLimit sent from " + "%s on %s (current = %d, received = %d). " + "Ignored.\n", ip6_sprintf(ip6bufs, &ip6->ip6_src), + if_name(ifp), ndi->chlim, nd_ra->nd_ra_curhoplimit); + } + } dr = defrtrlist_update(&dr0); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201504072020.t37KKl5Q032136>