CVE-2009-0159: NTP Remote Stack Overflow

NTP is a popular open source implementation of the NTP (Network Time Protocol). The bug on this CVE ID affects NTP prior to 4.2.4p7-RC2 release. The vulnerability was reported by Apple and disclosed on 30 March 2009. Here I’ll be using NTP 4.2.4p6 to demonstrate it.

3045 /*
3046  * cookedprint - output variables in cooked mode
3047  */
3048 static void
3049 cookedprint(
3050         int datatype,
3051         int length,
3052         char *data,
3053         int status,
3054         FILE *fp
3055         )
3056 {

This function can be found at ntpq/ntpq.c which is the source code file containing the NTP Queries routines. This automatically means that the bug discussed here can only be triggered by a malicious NTP response. Let’s examine the above function now…

3057         register int varid;
3058         char *name;
3059         char *value;
     ...
3061         int fmt;
3062         struct ctl_var *varlist;
     ...
3066         u_long uval;
     ...
3089         while (nextvar(&length, &data, &name, &value)) {
3090                 varid = findvar(name, varlist, 0);
3091                 if (varid == 0) {
     ...
3093                 } else {
3094                         output_raw = 0;
3095                         fmt = varlist[varid].fmt;
3096                         switch(fmt) {
     ...
3184                             case OC:
3185                                 if (!decodeuint(value, &uval))
3186                                     output_raw = '?';
3187                                 else {
3188                                         char b[10];
3189
3190                                         (void) sprintf(b, "%03lo", uval);
3191                                         output(fp, name, b);
3192                                 }
3193                                 break;
     ...
3240 }

So… at line 3089 it iterates using a while loop to retrieve the next variable of data buffer. Then, it invokes findvar() which attempts to identify if the previously returned variable (name) is known. Now, if findvar() doesn’t return 0 it’ll drop into the else clause at line 3093. At line 3095 it sets fmt to point to the appropriate variable from the ctl_var structure. If this format is 0C (line 3184) which means:

95 /*
96  * Format values
97  */
     ...
110 #define OC      12      /* integer, print in octal */

It will invoke decodeuint() to decode an unsigned integer from value and store the result into uval unsigned long. If this fails, it will jump into the else (line 3187) part where it declares a local buffer of 10 bytes size and then attempts to copy %03lo digits of the uval integer to it. This means: 03 Long Octal digits which gives a maximum of 11 digits not including the NULL termination. This leads to a possible off-by-two overflow on the above code since b is only 10 bytes long. The final call to output() at line 3191 simply outputs the name=b variable to fp. The fix of this bug was:

                  output_raw = '?';
              else {
-                     char b[10];
+                     char b[12];
-                     (void) sprintf(b, "%03lo", uval);
+                     (void) snprintf(b, sizeof(b), "%03lo",uval);
                      output(fp, name, b);
                    }

Which updates b[]‘s size to have sufficient space to store the integer, and also replaces sprintf() with the more secure snprintf() library routine.