[Bug 3612] Use-of-uninitialized-value in receive function

bk: 5d9ebf49XLWgi5TnxojMQQjg7bbp5w

diff --git a/ChangeLog b/ChangeLog
index dab3e21b5f93b0c7fcacc1117707287fd0248283..0233a06c08d2f544e3558b5745092426c9eff81e 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -5,6 +5,8 @@
 * [Bug 3615] accelerate refclock startup <perlinger@ntp.org>
 * [Bug 3613] Propagate noselect to mobilized pool servers <stenn@ntp.org>
   - Reported by Martin Burnicki
+* [Bug 3612] Use-of-uninitialized-value in receive function <perlinger@ntp.org>
+  - Reported by Philippe Antoine
 * [Bug 3611] NMEA time interpreted incorrectly <perlinger@ntp.org>
   - officially document new "trust date" mode bit for NMEA driver
   - restore the (previously undocumented) "trust date" feature lost with [bug 3577] 

diff --git a/ntpd/ntp_proto.c b/ntpd/ntp_proto.c
index c5c3d6c146ecc1bea2fa696f75b1d4583390e5d1..38a4682dfa9a58ea70b27414318c3a80440950fe 100644 (file)
--- a/ntpd/ntp_proto.c
+++ b/ntpd/ntp_proto.c
@@ -645,31 +645,20 @@ receive(
         */
        /*
         * Bogus port check is before anything, since it probably
-        * reveals a clogging attack.
+        * reveals a clogging attack. Likewise the mimimum packet size
+        * of 2 bytes (for mode 6/7) must be checked first.
         */
        sys_received++;
-       if (0 == SRCPORT(&rbufp->recv_srcadr)) {
+       if (0 == SRCPORT(&rbufp->recv_srcadr) || rbufp->recv_length < 2) {
                sys_badlength++;
-               return;                         /* bogus port */
+               return;                         /* bogus port / length */
        }
        restrictions(&rbufp->recv_srcadr, &r4a);
        restrict_mask = r4a.rflags;
 
        pkt = &rbufp->recv_pkt;
        hisversion = PKT_VERSION(pkt->li_vn_mode);
-       hisleap = PKT_LEAP(pkt->li_vn_mode);
        hismode = (int)PKT_MODE(pkt->li_vn_mode);
-       hisstratum = PKT_TO_STRATUM(pkt->stratum);
-       DPRINTF(1, ("receive: at %ld %s<-%s ippeerlimit %d mode %d iflags %s restrict %s org %#010x.%08x xmt %#010x.%08x\n",
-                   current_time, stoa(&rbufp->dstadr->sin),
-                   stoa(&rbufp->recv_srcadr), r4a.ippeerlimit, hismode,
-                   build_iflags(rbufp->dstadr->flags),
-                   build_rflags(restrict_mask),
-                   ntohl(pkt->org.l_ui), ntohl(pkt->org.l_uf),
-                   ntohl(pkt->xmt.l_ui), ntohl(pkt->xmt.l_uf)));
-
-       /* See basic mode and broadcast checks, below */
-       INSIST(0 != hisstratum);
 
        if (restrict_mask & RES_IGNORE) {
                DPRINTF(2, ("receive: drop: RES_IGNORE\n"));
@@ -701,6 +690,30 @@ receive(
                return;                         /* no time serve */
        }
 
+
+       /* If we arrive here, we should have a standard NTP packet. We
+        * check that the minimum size is available and fetch some more
+        * items from the packet once we can be sure they are indeed
+        * there.
+        */
+       if (rbufp->recv_length < LEN_PKT_NOMAC) {
+               sys_badlength++;
+               return;                         /* bogus length */
+       }
+
+       hisleap = PKT_LEAP(pkt->li_vn_mode);
+       hisstratum = PKT_TO_STRATUM(pkt->stratum);
+       INSIST(0 != hisstratum); /* paranoia check PKT_TO_STRATUM result */
+
+       DPRINTF(1, ("receive: at %ld %s<-%s ippeerlimit %d mode %d iflags %s "
+                   "restrict %s org %#010x.%08x xmt %#010x.%08x\n",
+                   current_time, stoa(&rbufp->dstadr->sin),
+                   stoa(&rbufp->recv_srcadr), r4a.ippeerlimit, hismode,
+                   build_iflags(rbufp->dstadr->flags),
+                   build_rflags(restrict_mask),
+                   ntohl(pkt->org.l_ui), ntohl(pkt->org.l_uf),
+                   ntohl(pkt->xmt.l_ui), ntohl(pkt->xmt.l_uf)));
+
        /*
         * This is for testing. If restricted drop ten percent of
         * surviving packets.