Perform length and type checking in ifchange.c:fill_options.c().

ndhc/ifchange.c

@@ -57,26 +57,39 @@ static int sprintip(char *dest, size_t size, char *pre, unsigned char *ip)
 
 /* Fill dest with the text of option 'option'. */
 /* Returns 0 if successful, -1 if nothing was filled in. */
-static int fill_options(char *dest, unsigned char *option,
+static int fill_options(char *dest, unsigned char *option, ssize_t optlen,
-						 struct dhcp_option *type_p, unsigned int maxlen)
+                        struct dhcp_option *type_p, unsigned int maxlen)
 {
-    int type, optlen;
+    char *odest;
-    uint16_t val_u16;
+    enum option_type type = type_p->type;
-    int16_t val_s16;
+    ssize_t typelen = option_length(type);
     uint32_t val_u32;
     int32_t val_s32;
-    char *odest;
+    uint16_t val_u16;
+    int16_t val_s16;
+    uint8_t code = type_p->code;
 
     if (!option)
         return -1;
+    if (optlen != typelen) {
+        if (option_valid_list(code)) {
+            if ((optlen % typelen)) {
+                log_warning("Bad data received - option list size mismatch: code=0x%02x proplen=0x%02x optlen=0x%02x",
+                            code, typelen, optlen);
+                return -1;
+            }
+        } else {
+            log_warning("Bad data received - option size mismatch: code=0x%02x proplen=0x%02x optlen=0x%02x",
+                        code, typelen, optlen);
+            return -1;
+        }
+    }
     int len = option[-1]; // XXX: WTF ugly as all hell
 
     odest = dest;
 
     dest += snprintf(dest, maxlen, "%s=", type_p->name);
 
-    type = type_p->type;
-    optlen = option_lengths[type];
     for(;;) {
         switch (type) {
             case OPTION_IP: /* Works regardless of host byte order. */
@@ -112,9 +125,11 @@ static int fill_options(char *dest, unsigned char *option,
                 memcpy(dest, option, len);
                 dest[len] = '\0';
                 return 0;  /* Short circuit this case */
+            case OPTION_NONE:
+                return 0;
         }
-        option += optlen;
+        option += typelen;
-        len -= optlen;
+        len -= typelen;
         if (len <= 0)
             break;
         *(dest++) = ':';
@@ -189,7 +204,7 @@ static void translate_option(int sockfd, struct dhcpMessage *packet,
     memset(buf2, '\0', sizeof(buf2));
 
     p = get_option(packet, code, &optlen);
-    if (fill_options(buf2, p, opt, sizeof buf2 - 1) == -1)
+    if (fill_options(buf2, p, optlen, opt, sizeof buf2 - 1) == -1)
         return;
     snprintf(buf, sizeof buf, "%s:", buf2);
     for (i = 0; i < 256; i++) {

ndhc/options.c

@@ -49,20 +49,57 @@ struct dhcp_option options[] = {
     {"maxsize"  ,   OPTION_U16,                             0x39},
     {"tftp"     ,   OPTION_STRING,                          0x42},
     {"bootfile" ,   OPTION_STRING,                          0x43},
-    {""         ,   0x00,                                   0x00}
+    {""         ,   OPTION_NONE,                            0x00}
 };
 
-/* Lengths of the different option types */
+// List of options that will be sent on the parameter request list to the
-int option_lengths[] = {
+// remote DHCP server.
-	[OPTION_IP] =       4,
+static unsigned char req_opts[] = {
-	[OPTION_STRING] =   1,
+	DHCP_SUBNET,
-	[OPTION_U8] =       1,
+	DHCP_ROUTER,
-	[OPTION_U16] =      2,
+	DHCP_DNS_SERVER,
-	[OPTION_S16] =      2,
+	DHCP_HOST_NAME,
-	[OPTION_U32] =      4,
+	DHCP_DOMAIN_NAME,
-	[OPTION_S32] =      4
+	DHCP_BROADCAST,
+	0x00
 };
 
+static unsigned char list_opts[] = {
+	DHCP_ROUTER,
+	DHCP_TIME_SERVER,
+	DHCP_NAME_SERVER,
+	DHCP_DNS_SERVER,
+	DHCP_LOG_SERVER,
+	DHCP_COOKIE_SERVER,
+	DHCP_LPR_SERVER,
+	DHCP_NTP_SERVER,
+	DHCP_WINS_SERVER,
+	0x00
+};
+
+uint8_t option_length(enum option_type type)
+{
+	switch (type) {
+		case OPTION_IP: return 4;
+		case OPTION_STRING: return 1; // XXX ?
+		case OPTION_U8: return 1;
+		case OPTION_U16: return 2;
+		case OPTION_S16: return 2;
+		case OPTION_U32: return 4;
+		case OPTION_S32: return 4;
+		default: return 0;
+	}
+}
+
+int option_valid_list(uint8_t code)
+{
+	int i;
+	for (i = 0; i < sizeof list_opts; ++i)
+		if (list_opts[i] == code)
+			return 1;
+	return 0;
+}
+
 size_t sizeof_option(unsigned char code, size_t datalen)
 {
 	if (code == DHCP_PADDING || code == DHCP_END)
@@ -229,7 +266,7 @@ int add_simple_option(unsigned char *optionptr, unsigned char code,
 
 	for (i = 0; options[i].code; i++)
 		if (options[i].code == code) {
-			length = option_lengths[(size_t)options[i].type];
+			length = option_length(options[i].type);
 		}
 
 	log_line("aso(): code=0x%02x length=0x%02x", code, length);
@@ -252,18 +289,6 @@ int add_simple_option(unsigned char *optionptr, unsigned char code,
 	return add_option_string(optionptr, option);
 }
 
-// List of options that will be sent on the parameter request list to the
-// remote DHCP server.
-static unsigned char req_opts[] = {
-	DHCP_SUBNET,
-	DHCP_ROUTER,
-	DHCP_DNS_SERVER,
-	DHCP_HOST_NAME,
-	DHCP_DOMAIN_NAME,
-	DHCP_BROADCAST,
-	0x00
-};
-
 /* Add a paramater request list for stubborn DHCP servers.  Don't do bounds */
 /* checking here because it goes towards the head of the packet. */
 void add_requests(struct dhcpMessage *packet)

ndhc/options.h

@@ -51,8 +51,9 @@
 #define DHCP_WPAD               0xfc
 #define DHCP_END                0xff
 
-enum {
+enum option_type {
-	OPTION_IP=1,
+	OPTION_NONE = 0,
+	OPTION_IP,
 	OPTION_STRING,
 	OPTION_U8,
 	OPTION_U16,
@@ -63,13 +64,14 @@ enum {
 
 struct dhcp_option {
 	char name[10];
-	char type;
+	enum option_type type;
 	unsigned char code;
 };
 
 extern struct dhcp_option options[];
-extern int option_lengths[];
 
+uint8_t option_length(enum option_type type);
+int option_valid_list(uint8_t code);
 size_t sizeof_option(unsigned char code, size_t datalen);
 size_t set_option(unsigned char *buf, size_t buflen, unsigned char code,
 				  unsigned char *optdata, size_t datalen);