[Sec 3565] null pointer crash by remote attack

bk: 5c3f9753-uhxXmYRwCTIgy-6GT15_g

diff --git a/ChangeLog b/ChangeLog
index e342adf2a47bd938d3d67c7edea9c8bb61928889..1c835e70785c533ea9e3e0c29488f767c8d5e102 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,6 @@
+---
+* [Sec 3565] null pointer crash by remote attack <perlinger@ntp.org>
+
 ---
 (4.2.8p13)
 

diff --git a/ntpd/ntp_control.c b/ntpd/ntp_control.c
index 63f9980ac27c0c20ffe9ad24304fd66961ec474c..49a197ed7c61ccf906a2d76d4070063abed8b3a7 100644 (file)
--- a/ntpd/ntp_control.c
+++ b/ntpd/ntp_control.c
@@ -3448,11 +3448,11 @@ write_variables(
         * Look through the variables. Dump out at the first sign of
         * trouble.
         */
-       while ((v = ctl_getitem(sys_var, &valuep)) != 0) {
+       while ((v = ctl_getitem(sys_var, &valuep)) != NULL) {
                ext_var = 0;
                if (v->flags & EOV) {
-                       if ((v = ctl_getitem(ext_sys_var, &valuep)) !=
-                           0) {
+                       v = ctl_getitem(ext_sys_var, &valuep);
+                       if (v != NULL) {
                                if (v->flags & EOV) {
                                        ctl_error(CERR_UNKNOWNVAR);
                                        return;
@@ -3466,16 +3466,24 @@ write_variables(
                        ctl_error(CERR_PERMISSION);
                        return;
                }
-               if (!ext_var && (*valuep == '\0' || !atoint(valuep,
-                                                           &val))) {
+               /* [bug 3565] writing makes sense only if we *have* a
+                * value in the packet!
+                */
+               if (valuep == NULL) {
                        ctl_error(CERR_BADFMT);
                        return;
                }
-               if (!ext_var && (val & ~LEAP_NOTINSYNC) != 0) {
-                       ctl_error(CERR_BADVALUE);
-                       return;
+               if (!ext_var) {
+                       if ( !(*valuep && atoint(valuep, &val))) {
+                               ctl_error(CERR_BADFMT);
+                               return;
+                       }
+                       if ((val & ~LEAP_NOTINSYNC) != 0) {
+                               ctl_error(CERR_BADVALUE);
+                               return;
+                       }
                }
-
+               
                if (ext_var) {
                        octets = strlen(v->text) + strlen(valuep) + 2;
                        vareqv = emalloc(octets);