[Sec 2671] vallen in extension fields are not validated

bk: 54a7c595jlwS3KmAxBML75HFGLR_pQ

diff --git a/ChangeLog b/ChangeLog
index afd18dae6c292327dfb94c9b051b07d5083b7791..441dc7bcf98fe9350da2bcc46b17934ea75c2cb9 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,7 @@
 ---
+
+* [Sec 2671] vallen in extension fields are not validated.
+---
 (4.2.8p1-beta3) 2015/01/02 Released by Harlan Stenn <stenn@ntp.org>
 
 * [Bug 2627] shm refclock allows only two units with owner-only access

diff --git a/ntpd/ntp_crypto.c b/ntpd/ntp_crypto.c
index 4f27ab8bd5ccb13c96fb7635fe4d3295781675e4..f8093add224a350a65e2bade09e84372c40a8367 100644 (file)
--- a/ntpd/ntp_crypto.c
+++ b/ntpd/ntp_crypto.c
@@ -1194,7 +1194,8 @@ crypto_xmit(
         */
        case CRYPTO_CERT | CRYPTO_RESP:
                vallen = ntohl(ep->vallen);
-               if (vallen == 0 || vallen > MAXHOSTNAME) {
+               if (vallen == 0 || vallen > MAXHOSTNAME ||
+                   len < VALUE_LEN + vallen) {
                        rval = XEVNT_LEN;
                        break;
                }
@@ -1344,7 +1345,9 @@ crypto_xmit(
         * anything goes wrong.
         */
        case CRYPTO_COOK | CRYPTO_RESP:
-               if ((opcode & 0xffff) < VALUE_LEN) {
+               vallen = ntohl(ep->vallen);
+               if (vallen == 0
+                   || (opcode & 0x0000ffff)  < VALUE_LEN + vallen) {
                        rval = XEVNT_LEN;
                        break;
                }
@@ -2165,7 +2168,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
@@ -2180,8 +2184,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 + vallen)
+               return XEVNT_LEN;
+       if ((r = BN_bin2bn((u_char *)ep->pkt, vallen, NULL)) == NULL) {
                msyslog(LOG_ERR, "crypto_bob: %s",
                    ERR_error_string(ERR_get_error(), NULL));
                return (XEVNT_ERR);
@@ -2193,7 +2200,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 */
@@ -2212,8 +2219,8 @@ crypto_bob(
         * Encode the values in ASN.1 and sign. The filestamp is from
         * the local file.
         */
-       len = i2d_DSA_SIG(sdsa, NULL);
-       if (len == 0) {
+       vallen = i2d_DSA_SIG(sdsa, NULL);
+       if (vallen == 0) {
                msyslog(LOG_ERR, "crypto_bob: %s",
                    ERR_error_string(ERR_get_error(), NULL));
                DSA_SIG_free(sdsa);
@@ -2223,8 +2230,8 @@ crypto_bob(
        tstamp = crypto_time();
        vp->tstamp = htonl(tstamp);
        vp->fstamp = htonl(iffkey_info->fstamp);
-       vp->vallen = htonl(len);
-       ptr = emalloc(len);
+       vp->vallen = htonl(vallen);
+       ptr = emalloc(vallen);
        vp->ptr = ptr;
        i2d_DSA_SIG(sdsa, &ptr);
        DSA_SIG_free(sdsa);
@@ -2234,8 +2241,8 @@ crypto_bob(
        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(sign_siglen);
        return (XEVNT_OK);
 }