In dhclient check the data for some string options for

reasonableness before passing it along to the script that
interfaces with the OS. [ISC-Bugs #23722] CVE: CVE-2011-0997

diff --git a/RELNOTES b/RELNOTES
index 3693403496f1c67faa98d0b357f1e456dba03bc5..40b1ab690e01ad328b1a778758205f9654528b15 100644 (file)
--- a/RELNOTES
+++ b/RELNOTES
@@ -39,6 +39,14 @@ The system has only been tested on Linux, FreeBSD, and Solaris, and may not
 work on other platforms. Please report any problems and suggested fixes to
 <dhcp-users@isc.org>.
 
+                       Changes since 4.2.1
+
+! In dhclient check the data for some string options for
+  reasonableness before passing it along to the script that
+  interfaces with the OS.
+  [ISC-Bugs #23722]
+  CVE: CVE-2011-0997
+  
                        Changes since 4.2.1rc1
 
 - None

diff --git a/client/dhclient.c b/client/dhclient.c
index dc19e8be81d9a8f38aa5f76a35b2b1d2254689f8..970b93554b62913c7abe77d657a71e418a76986f 100644 (file)
--- a/client/dhclient.c
+++ b/client/dhclient.c
@@ -91,6 +91,11 @@ static void usage(void);
 
 static isc_result_t write_duid(struct data_string *duid);
 
+static int check_domain_name(const char *ptr, size_t len, int dots);
+static int check_domain_name_list(const char *ptr, size_t len, int dots);
+static int check_option_values(struct universe *universe, unsigned int opt,
+                              const char *ptr, size_t len);
+
 int
 main(int argc, char **argv) {
        int fd;
@@ -3034,13 +3039,23 @@ void client_option_envadd (struct option_cache *oc,
                if (data.len) {
                        char name [256];
                        if (dhcp_option_ev_name (name, sizeof name,
-                                                oc -> option)) {
-                               client_envadd (es -> client, es -> prefix,
-                                              name, "%s",
-                                              (pretty_print_option
-                                               (oc -> option,
-                                                data.data, data.len,
-                                                0, 0)));
+                                                oc->option)) {
+                               const char *value;
+                               value = pretty_print_option(oc->option,
+                                                           data.data,
+                                                           data.len, 0, 0);
+                               size_t length = strlen(value);
+
+                               if (check_option_values(oc->option->universe,
+                                                       oc->option->code,
+                                                       value, length) == 0) {
+                                       client_envadd(es->client, es->prefix,
+                                                     name, "%s", value);
+                               } else {
+                                       log_error("suspect value in %s "
+                                                 "option - discarded",
+                                                 name);
+                               }
                                data_string_forget (&data, MDL);
                        }
                }
@@ -3118,12 +3133,32 @@ void script_write_params (client, prefix, lease)
                data_string_forget (&data, MDL);
        }
 
-       if (lease -> filename)
-               client_envadd (client,
-                              prefix, "filename", "%s", lease -> filename);
-       if (lease -> server_name)
-               client_envadd (client, prefix, "server_name",
-                              "%s", lease -> server_name);
+       if (lease->filename) {
+               if (check_option_values(NULL, DHO_ROOT_PATH,
+                                       lease->filename,
+                                       strlen(lease->filename)) == 0) {
+                       client_envadd(client, prefix, "filename",
+                                     "%s", lease->filename);
+               } else {
+                       log_error("suspect value in %s "
+                                 "option - discarded",
+                                 lease->filename);
+               }
+       }
+
+       if (lease->server_name) {
+               if (check_option_values(NULL, DHO_HOST_NAME,
+                                       lease->server_name,
+                                       strlen(lease->server_name)) == 0 ) {
+                       client_envadd (client, prefix, "server_name",
+                                      "%s", lease->server_name);
+               } else {
+                       log_error("suspect value in %s "
+                                 "option - discarded",
+                                 lease->server_name);
+               }
+       }
+                               
 
        for (i = 0; i < lease -> options -> universe_count; i++) {
                option_space_foreach ((struct packet *)0, (struct lease *)0,
@@ -4026,3 +4061,128 @@ dhcpv4_client_assignments(void)
        } else
                remote_port = htons (ntohs (local_port) - 1);   /* XXX */
 }
+
+/*
+ * The following routines are used to check that certain
+ * strings are reasonable before we pass them to the scripts.
+ * This avoids some problems with scripts treating the strings
+ * as commands - see ticket 23722
+ * The domain checking code should be done as part of assembling
+ * the string but we are doing it here for now due to time
+ * constraints.
+ */
+
+static int check_domain_name(const char *ptr, size_t len, int dots)
+{
+       const char *p;
+
+       /* not empty or complete length not over 255 characters   */
+       if ((len == 0) || (len > 256))
+               return(-1);
+
+       /* consists of [[:alnum:]-]+ labels separated by [.]      */
+       /* a [_] is against RFC but seems to be "widely used"...  */
+       for (p=ptr; (*p != 0) && (len-- > 0); p++) {
+               if ((*p == '-') || (*p == '_')) {
+                       /* not allowed at begin or end of a label */
+                       if (((p - ptr) == 0) || (len == 0) || (p[1] == '.'))
+                               return(-1);
+               } else if (*p == '.') {
+                       /* each label has to be 1-63 characters;
+                          we allow [.] at the end ('foo.bar.')   */
+                       size_t d = p - ptr;
+                       if ((d <= 0) || (d >= 64))
+                               return(-1);
+                       ptr = p + 1; /* jump to the next label    */
+                       if ((dots > 0) && (len > 0))
+                               dots--;
+               } else if (isalnum((unsigned char)*p) == 0) {
+                       /* also numbers at the begin are fine     */
+                       return(-1);
+               }
+       }
+       return(dots ? -1 : 0);
+}
+
+static int check_domain_name_list(const char *ptr, size_t len, int dots)
+{
+       const char *p;
+       int ret = -1; /* at least one needed */
+
+       if ((ptr == NULL) || (len == 0))
+               return(-1);
+
+       for (p=ptr; (*p != 0) && (len > 0); p++, len--) {
+               if (*p != ' ')
+                       continue;
+               if (p > ptr) {
+                       if (check_domain_name(ptr, p - ptr, dots) != 0)
+                               return(-1);
+                       ret = 0;
+               }
+               ptr = p + 1;
+       }
+       if (p > ptr)
+               return(check_domain_name(ptr, p - ptr, dots));
+       else
+               return(ret);
+}
+
+static int check_option_values(struct universe *universe,
+                              unsigned int opt,
+                              const char *ptr,
+                              size_t len)
+{
+       if (ptr == NULL)
+               return(-1);
+
+       /* just reject options we want to protect, will be escaped anyway */
+       if ((universe == NULL) || (universe == &dhcp_universe)) {
+               switch(opt) {
+                     case DHO_HOST_NAME:
+                     case DHO_DOMAIN_NAME:
+                     case DHO_NIS_DOMAIN:
+                     case DHO_NETBIOS_SCOPE:
+                       return check_domain_name(ptr, len, 0);
+                       break;
+                     case DHO_DOMAIN_SEARCH:
+                       return check_domain_name_list(ptr, len, 0);
+                       break;
+                     case DHO_ROOT_PATH:
+                       if (len == 0)
+                               return(-1);
+                       for (; (*ptr != 0) && (len-- > 0); ptr++) {
+                               if(!(isalnum((unsigned char)*ptr) ||
+                                    *ptr == '#'  || *ptr == '%' ||
+                                    *ptr == '+'  || *ptr == '-' ||
+                                    *ptr == '_'  || *ptr == ':' ||
+                                    *ptr == '.'  || *ptr == ',' ||
+                                    *ptr == '@'  || *ptr == '~' ||
+                                    *ptr == '\\' || *ptr == '/' ||
+                                    *ptr == '['  || *ptr == ']' ||
+                                    *ptr == '='  || *ptr == ' '))
+                                       return(-1);
+                       }
+                       return(0);
+                       break;
+               }
+       }
+
+#ifdef DHCPv6
+       if (universe == &dhcpv6_universe) {
+               switch(opt) {
+                     case D6O_SIP_SERVERS_DNS:
+                     case D6O_DOMAIN_SEARCH:
+                     case D6O_NIS_DOMAIN_NAME:
+                     case D6O_NISP_DOMAIN_NAME:
+                       return check_domain_name_list(ptr, len, 0);
+                       break;
+               }
+       }
+#endif
+
+       return(0);
+}
+ 
+
+

diff --git a/common/options.c b/common/options.c
index 28c36e6be1be49c108a83dadba52a5a1f4c452b6..167061d59813c4447858b80b85629229c48a6654 100644 (file)
--- a/common/options.c
+++ b/common/options.c
@@ -3915,7 +3915,8 @@ pretty_escape(char **dst, char *dend, const unsigned char **src,
                                count += 4;
                        }
                } else if (**src == '"' || **src == '\'' || **src == '$' ||
-                          **src == '`' || **src == '\\') {
+                          **src == '`' || **src == '\\' || **src == '|' ||
+                          **src == '&') {
                        if (*dst + 2 > dend)
                                return -1;