system/5158: patches that I think really need to be put in place

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

system/5158: patches that I think really need to be put in place

Peter J. Philipp
>Number:         5158
>Category:       system
>Synopsis:       dhclient patches, needed
>Confidential:   yes
>Severity:       critical
>Priority:       high
>Responsible:    bugs
>State:          open
>Quarter:        
>Keywords:      
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Jun 21 22:50:01 GMT 2006
>Closed-Date:
>Last-Modified:
>Originator:     Peter Philipp
>Release:        3.9-REL
>Organization:
net
>Environment:
       
        System      : OpenBSD 3.9
        Architecture: OpenBSD.amd64
        Machine     : amd64
>Description:
        I got the gnomes... ... ... :( ... :( ... :(
        The patches do the following.. 1) instead of blacklisting 2 illegal
        characters in dhclient.c they whitelist a list of characters.
        2) dhclient-script checks throguh regex in a case/esac statement
           whether the input is really an IP string (whitespaces or dotted
                quads).
        3) some extra checking with if test cases ala /etc/rc      

        this needs reading over, I can't really test right now, not sure
        what else is compromised. :( (it compiles though)

>How-To-Repeat:
        Just be me and get people helping you along...messing with my
        sanity. *cry*
>Fix:
--- dhclient-script.orig Thu Jun 22 00:22:39 2006
+++ dhclient-script Thu Jun 22 00:22:49 2006
@@ -61,6 +61,17 @@
  route delete default >/dev/null 2>&1
 
  if [ -n "$old_static_routes" ]; then
+ #
+ # these should only contain whitespaces, dots and numbers
+ #
+ case recheck in $new_static_routes
+ '[0-9\. ]*')
+ ;;
+ *)
+ return 1;
+ ;;
+ esac
+
  set $old_static_routes
  while [ $# -gt 1 ]; do
  route delete "$1" "$2"
@@ -74,7 +85,7 @@
 add_new_routes() {
  route delete default >/dev/null 2>&1
  for router in $new_routers; do
- if [ "$new_ip_address" = "$router" ]; then
+ if [ X"$new_ip_address" = X"$router" ]; then
  route add default -iface $router >/dev/null 2>&1
  else
  route add default $router >/dev/null 2>&1
@@ -85,6 +96,17 @@
  done
 
  if [ -n "$new_static_routes" ]; then
+ #
+ # these should only contain whitespaces, dots and numbers
+ #
+ case recheck in $new_static_routes
+ '[0-9\. ]*')
+ ;;
+ *)
+ return 1;
+ ;;
+ esac
+
  set $new_static_routes
  while [ $# -gt 1 ]; do
  route add $1 $2
@@ -165,22 +187,22 @@
 
 BOUND|RENEW|REBIND|REBOOT)
  if [ -n "$old_ip_address" ]; then
- if [ "$old_ip_address" != "$alias_ip_address" ]; then
+ if [ X"$old_ip_address" != X"$alias_ip_address" ]; then
  delete_old_alias
  fi
- if [ "$old_ip_address" != "$new_ip_address" ]; then
+ if [ X"$old_ip_address" != X"$new_ip_address" ]; then
  delete_old_address
  delete_old_routes
  fi
  fi
- if [ "$reason" = BOUND ] || \
+ if [ X"$reason" = XBOUND ] || \
    [ "$reason" = REBOOT ] || \
    [ -z "$old_ip_address" ] || \
    [ "$old_ip_address" != "$new_ip_address" ]; then
  add_new_address
  add_new_routes
  fi
- if [ "$new_ip_address" != "$alias_ip_address" ]; then
+ if [ X"$new_ip_address" != X"$alias_ip_address" ]; then
  add_new_alias
  fi
  add_new_resolv_conf
@@ -204,9 +226,21 @@
  add_new_address
  sleep 1
  if [ -n "$new_routers" ]; then
+ #
+ # these should only contain whitespaces, dots and numbers
+ #
+ case recheck in $new_static_routes
+ '[0-9\. ]*')
+ ;;
+ *)
+ return 1;
+ ;;
+ esac
+
+
  set "$new_routers"
  if ping -q -c 1 -w 1 "$1"; then
- if [ "$new_ip_address" != "$alias_ip_address" ]; then
+ if [ X"$new_ip_address" != X"$alias_ip_address" ]; then
  add_new_alias
  fi
  add_new_routes
--- dhclient.c.orig Wed Oct 26 17:42:04 2005
+++ dhclient.c Thu Jun 22 00:35:02 2006
@@ -66,6 +66,8 @@
 char *path_dhclient_conf = _PATH_DHCLIENT_CONF;
 char *path_dhclient_db = NULL;
 
+char allowable_characters[] = "0123456789!@#%^()_ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz\\/.,:'\"";
+
 int log_perror = 1;
 int privfd;
 int nullfd = -1;
@@ -1961,6 +1963,7 @@
 {
  struct client_state *client = ifi->client;
  int i, j, namelen;
+ int allowchar;
 
  namelen = strlen(name);
 
@@ -2001,12 +2004,15 @@
 
  /* No `` or $() command substitution allowed in environment values! */
  for (j = 0; j < strlen(value); j++)
- switch (value[j]) {
- case '`':
- case '$':
- error("illegal character (%c) in value '%s'", value[j],
-    value);
- /* not reached */
+ for (allowchar = 0; allowchar < strlen(allowable_characters); allowchar++) {
+ if (value[j] == allowable_characters[allowchar])
+ continue;
+ /* if the character is not in there... */
+
+ error("illegal character (0x%02X) in value '%s'",
+ value[j] & 0xff, value);
+
+ /* NOTREACHED */
  }
  snprintf(client->scriptEnv[i], strlen(prefix) + strlen(name) +
     1 + strlen(value) + 1, "%s%s=%s", prefix, name, value);


>Release-Note:
>Audit-Trail:
>Unformatted:

Reply | Threaded
Open this post in threaded view
|

Re: system/5158: patches that I think really need to be put in place

Peter J. Philipp
The following reply was made to PR system/5158; it has been noted by GNATS.

From: Peter Philipp <[hidden email]>
To: [hidden email]
Cc:  
Subject: Re: system/5158: patches that I think really need to be put in place
Date: Thu, 22 Jun 2006 01:43:59 +0200

 > --- dhclient-script.orig Thu Jun 22 00:22:39 2006
 > +++ dhclient-script Thu Jun 22 00:22:49 2006
 > @@ -61,6 +61,17 @@
 >   route delete default >/dev/null 2>&1
 >  
 >   if [ -n "$old_static_routes" ]; then
 > + #
 > + # these should only contain whitespaces, dots and numbers
 > + #
 > + case recheck in $new_static_routes
 
 Whoops, sorry for this.. bad case of copy-pasting...  here is the real
 patch, untested again..
 
 Also if I didn't make it too clear these are against 3.9-RELEASE since I
 reinstalled off cdrom media that I bought from the OpenBSD Project.
 Bedtime for me now.. night.
 
 
 --- dhclient-script.orig Thu Jun 22 00:22:39 2006
 +++ dhclient-script Thu Jun 22 01:40:20 2006
 @@ -61,6 +61,17 @@
  route delete default >/dev/null 2>&1
 
  if [ -n "$old_static_routes" ]; then
 + #
 + # these should only contain whitespaces, dots and numbers
 + #
 + case recheck in $old_static_routes
 + '[0-9\. ]*')
 + ;;
 + *)
 + return 1;
 + ;;
 + esac
 +
  set $old_static_routes
  while [ $# -gt 1 ]; do
  route delete "$1" "$2"
 @@ -74,7 +85,7 @@
  add_new_routes() {
  route delete default >/dev/null 2>&1
  for router in $new_routers; do
 - if [ "$new_ip_address" = "$router" ]; then
 + if [ X"$new_ip_address" = X"$router" ]; then
  route add default -iface $router >/dev/null 2>&1
  else
  route add default $router >/dev/null 2>&1
 @@ -85,6 +96,17 @@
  done
 
  if [ -n "$new_static_routes" ]; then
 + #
 + # these should only contain whitespaces, dots and numbers
 + #
 + case recheck in $new_static_routes
 + '[0-9\. ]*')
 + ;;
 + *)
 + return 1;
 + ;;
 + esac
 +
  set $new_static_routes
  while [ $# -gt 1 ]; do
  route add $1 $2
 @@ -165,22 +187,22 @@
 
  BOUND|RENEW|REBIND|REBOOT)
  if [ -n "$old_ip_address" ]; then
 - if [ "$old_ip_address" != "$alias_ip_address" ]; then
 + if [ X"$old_ip_address" != X"$alias_ip_address" ]; then
  delete_old_alias
  fi
 - if [ "$old_ip_address" != "$new_ip_address" ]; then
 + if [ X"$old_ip_address" != X"$new_ip_address" ]; then
  delete_old_address
  delete_old_routes
  fi
  fi
 - if [ "$reason" = BOUND ] || \
 + if [ X"$reason" = XBOUND ] || \
    [ "$reason" = REBOOT ] || \
    [ -z "$old_ip_address" ] || \
    [ "$old_ip_address" != "$new_ip_address" ]; then
  add_new_address
  add_new_routes
  fi
 - if [ "$new_ip_address" != "$alias_ip_address" ]; then
 + if [ X"$new_ip_address" != X"$alias_ip_address" ]; then
  add_new_alias
  fi
  add_new_resolv_conf
 @@ -204,9 +226,21 @@
  add_new_address
  sleep 1
  if [ -n "$new_routers" ]; then
 + #
 + # these should only contain whitespaces, dots and numbers
 + #
 + case recheck in $new_routers
 + '[0-9\. ]*')
 + ;;
 + *)
 + return 1;
 + ;;
 + esac
 +
 +
  set "$new_routers"
  if ping -q -c 1 -w 1 "$1"; then
 - if [ "$new_ip_address" != "$alias_ip_address" ]; then
 + if [ X"$new_ip_address" != X"$alias_ip_address" ]; then
  add_new_alias
  fi
  add_new_routes