File systemtap-CVE-2009-4273.diff of Package systemtap

Overview Repositories Revisions Requests Users Attributes Meta

File systemtap-CVE-2009-4273.diff of Package systemtap (Revision 29904bf9b6078543678b15b68c2117ef)

Currently displaying revision 29904bf9b6078543678b15b68c2117ef , Show latest

Subject: VUL-0: systemtap-server code exec
References: bnc#574243
Signed-Off-By: Tony Jones <tonyj@suse.de>

Fix remaining portions of this CVE (part2) still present post release-1.1
This second part is also referred to as CVE-2010-0412

Covers cases such as stap-client -B CC=/bin/rm CFLAGS=/ ...  where the 
execution is done by make

commit b75067caf1bb416af21473e40c917d953531e9f9
Author: Dave Brolley <brolley@redhat.com>
Date:   Mon Jan 18 11:56:13 2010 -0500

Correct client-side quoting issues discovered by fche during the server-side reimplementation.
    
    Also add the test cases to the test suite.

commit 241443ad36a5a2cacb9e8e6f12f808d304835f2a
Author: Dave Brolley <brolley@redhat.com>
Date:   Tue Feb 2 08:26:01 2010 -0500

PR 11105: Remaining client-side problems:
    
    stap-client: Correct handling of embedded newlines in arguments.
    server_args.exp: Add additional cases discovered by fche and by fuzzing.

commit 4240be911e37b817727ecb33f321f0ea389ede61
Author: Dave Brolley <brolley@redhat.com>
Date:   Mon Feb 15 15:01:28 2010 -0500

Don't pass client-only options to the server.
    
    Also correct parsing of the --server option.

commit d2334a2233f4efd055dab021c603f7c046730a66
Author: Dave Brolley <brolley@redhat.com>
Date:   Tue Feb 2 14:08:31 2010 -0500

Compile server logging and robustness.
    
    Log certificate location and status when starting server.
    Additional care in handling arguments in stap-serverd.
    New test case discovered by fuzzing added and fixed.

commit c0d1b5a004b9949bb455b7dbe17b335b7cab9ead
Author: Frank Ch. Eigler <fche@elastic.org>
Date:   Fri Feb 12 10:25:43 2010 -0500

PR11105 part 2: tighten constraints on stap-server parameters passed to make
    
    * util.h, util.cxx (assert_match_regexp): New function.
    * main.cxx (main): Constrain -R, -r, -a, -D, -S, -q, -B flags.
    * stap-serverd (listen): Harden stap-server-connect with ulimit/loop.
    * testsuite/systemtap.server/{client,server}_args.exp: Revised.

commit cc9e5488d82b728e568bca1f8d6094856fc8e641
Author: Frank Ch. Eigler <fche@elastic.org>
Date:   Fri Feb 12 10:39:58 2010 -0500

PR11105 part 2a, fix buggy \\. in -r option regexp

--- a/main.cxx
+++ b/main.cxx
@@ -57,7 +57,7 @@ version ()
     << "SystemTap translator/driver "
     << "(version " << VERSION << "/" << dwfl_version (NULL)
     << " " << GIT_MESSAGE << ")" << endl
-    << "Copyright (C) 2005-2009 Red Hat, Inc. and others" << endl
+    << "Copyright (C) 2005-2010 Red Hat, Inc. and others" << endl
     << "This is free software; see the source for copying conditions." << endl;
 }
 
@@ -670,12 +670,12 @@ main (int argc, char * const argv [])
           break;
 
         case 'o':
+          // NB: client_options not a problem, since pass 1-4 does not use output_file.
           s.output_file = string (optarg);
           break;
 
         case 'R':
-	  if (client_options)
-	    client_options_disallowed += client_options_disallowed.empty () ? "-R" : ", -R";
+          if (client_options) { cerr << "ERROR: -R invalid with --client-options" << endl; usage(s,1); }
           s.runtime_path = string (optarg);
           break;
 
@@ -684,6 +684,7 @@ main (int argc, char * const argv [])
 	    client_options_disallowed += client_options_disallowed.empty () ? "-m" : ", -m";
           s.module_name = string (optarg);
 	  save_module = true;
+          // XXX: convert to assert_regexp_match()
 	  {
 	    string::size_type len = s.module_name.length();
 
@@ -728,15 +729,14 @@ main (int argc, char * const argv [])
           break;
 
         case 'r':
-	  if (client_options)
-	    client_options_disallowed += client_options_disallowed.empty () ? "-r" : ", -r";
+          if (client_options) // NB: no paths!
+            assert_regexp_match("-r parameter from client", optarg, "^[a-z0-9_.-]+$");
           setup_kernel_release(s, optarg);
           break;
 
         case 'a':
-	  if (client_options)
-	  client_options_disallowed += client_options_disallowed.empty () ? "-a" : ", -a";
-	    s.architecture = string(optarg);
+          assert_regexp_match("-a parameter", optarg, "^[a-z0-9_-]+$");
+          s.architecture = string(optarg);
           break;
 
         case 'k':
@@ -783,16 +783,19 @@ main (int argc, char * const argv [])
 	  break;
 
 	case 'D':
+          assert_regexp_match ("-D parameter", optarg, "^[a-z_][a-z_0-9]*(=[a-z_0-9]+)?$");
 	  if (client_options)
 	    client_options_disallowed += client_options_disallowed.empty () ? "-D" : ", -D";
 	  s.macros.push_back (string (optarg));
 	  break;
 
 	case 'S':
+          assert_regexp_match ("-S parameter", optarg, "^[0-9]+(,[0-9]+)?$");
 	  s.size_option = string (optarg);
 	  break;
 
 	case 'q':
+          if (client_options) { cerr << "ERROR: -q invalid with --client-options" << endl; usage(s,1); }
 	  s.tapset_compile_coverage = true;
 	  break;
 
@@ -823,9 +826,8 @@ main (int argc, char * const argv [])
 	  break;
 
 	case 'B':
-	  if (client_options)
-	  client_options_disallowed += client_options_disallowed.empty () ? "-B" : ", -B";
-	    s.kbuildflags.push_back (string (optarg));
+          if (client_options) { cerr << "ERROR: -B invalid with --client-options" << endl; usage(s,1); }
+          s.kbuildflags.push_back (string (optarg));
 	  break;
 
         case 0:
--- a/stap-client
+++ b/stap-client
@@ -2,7 +2,7 @@
 
 # Compile server client for systemtap
 #
-# Copyright (C) 2008, 2009, 2010 Red Hat Inc.
+# Copyright (C) 2008-2010 Red Hat Inc.
 #
 # This file is part of systemtap, and is free software.  You can
 # redistribute it and/or modify it under the terms of the GNU General
@@ -84,34 +84,34 @@ function parse_options {
     # Each command line argument will be written to its own file within the
     # request package.
     argc=1
+    packed_options='-'
     arg_subst=
 
     while test $# != 0
     do
 	advance=0
 	dash_seen=0
+	client_arg=0
 
         # Start of a new token.
 	first_token="$1"
 	until test $advance != 0
 	do
             # Identify the next option
-	    first_char=`expr "$first_token" : '$.$.*'`
+	    first_char="${first_token:0:1}"
 	    second_char=
 	    if test $dash_seen = 0; then
 		if test "$first_char" = "-"; then
 		    if test "$first_token" != "-"; then
 	                # It's not a lone dash, so it's an option.
 			# Is it a long option (i.e. --option)?
-			second_char=`expr "$first_token" : '.$.$.*'`
+			second_char="${first_token:1:1}"
 			if test "X$second_char" = "X-"; then
-			    long_option=`expr "$first_token" : '--$.*$=.*'`
-			    test "X$long_option" != "X" || long_option=`expr "$first_token" : '--$.*$'`
-			    case $long_option in
-				ssl)
+			    case "$first_token" in
+				--ssl=*)
 				    process_ssl "$first_token"
 				    ;;
-				server)
+				--server=*)
 				    process_server "$first_token"
 				    ;;
 				*)
@@ -123,9 +123,9 @@ function parse_options {
 			fi
 	                # It's not a lone dash, or a long option, so it's a short option string.
 			# Remove the dash.
-			first_token=`expr "$first_token" : '-$.*$'`
+			first_token="${first_token:1}"
 			dash_seen=1
-			first_char=`expr "$first_token" : '$.$.*'`
+			first_char="${first_token:0:1}"
 		    fi
 		fi
 		if test $dash_seen = 0; then
@@ -145,69 +145,69 @@ function parse_options {
             # We are at the start of an option. Look at the first character.
 	    case $first_char in
 		a)
-		    get_arg $first_token "$2"
+		    get_arg "$first_token" "$2"
 		    process_a "$stap_arg"
 		    ;;	
 		B)
-		    get_arg $first_token "$2"
+		    get_arg "$first_token" "$2"
 		    ;;	
 		c)
-		    get_arg $first_token "$2"
+		    get_arg "$first_token" "$2"
 		    process_c "$stap_arg"
 		    ;;
 		D)
-		    get_arg $first_token "$2"
+		    get_arg "$first_token" "$2"
 		    ;;
 		e)
-		    get_arg $first_token "$2"
+		    get_arg "$first_token" "$2"
 		    process_e "$stap_arg"
 		    ;;
 		I)
-		    get_arg $first_token "$2"
+		    get_arg "$first_token" "$2"
 		    process_I "$stap_arg"
 		    ;;	
 		k)
 		    keep_temps=1
 		    ;;
 		l)
-		    get_arg $first_token "$2"
+		    get_arg "$first_token" "$2"
 		    p_phase=2
 		    ;;
 		L)
-		    get_arg $first_token "$2"
+		    get_arg "$first_token" "$2"
 		    p_phase=2
 		    ;;
 		m)
-		    get_arg $first_token "$2"
+		    get_arg "$first_token" "$2"
 		    process_m "$stap_arg"
 		    ;;
 		o)
-		    get_arg $first_token "$2"
+		    get_arg "$first_token" "$2"
 		    process_o "$stap_arg"
 		    ;;
 		p)
-		    get_arg $first_token "$2"
+		    get_arg "$first_token" "$2"
 		    process_p "$stap_arg"
 		    ;;
 		r)
-		    get_arg $first_token "$2"
+		    get_arg "$first_token" "$2"
 		    process_r "$stap_arg"
 		    ;;	
 		R)
-		    get_arg $first_token "$2"
+		    get_arg "$first_token" "$2"
 		    process_R "$stap_arg"
 		    ;;	
 		s)
-		    get_arg $first_token "$2"
+		    get_arg "$first_token" "$2"
 		    ;;	
 		S)
-		    get_arg $first_token "$2"
+		    get_arg "$first_token" "$2"
 		    ;;	
 		v)
 		    v_level=$(($v_level + 1))
 		    ;;
 		x)
-		    get_arg $first_token "$2"
+		    get_arg "$first_token" "$2"
 		    ;;
 		*)
 		    # An unknown or unimportant flag.
@@ -216,7 +216,7 @@ function parse_options {
 
 	    if test $advance = 0; then
 	        # Just another flag character. Consume it.
-		first_token=`expr "$first_token" : '.$.*$'`
+		first_token="${first_token:1}"
 		if test "X$first_token" = "X"; then
 		    advance=$(($advance + 1))
 		fi
@@ -224,9 +224,19 @@ function parse_options {
 	done
 
         # Consume the arguments we just processed.
-	while test $advance != 0
-	do
-	    # Place the argument is a numbered file within our temp
+	while test $advance != 0; do
+	    # Does the final argument file contain a client-side file
+	    # name which must be changed to a server-side name?
+	    local arg
+	    if test "X$arg_subst" != "X" -a $advance = 1; then
+		arg="$arg_subst"
+		arg_subst=
+	    else
+		arg="$1"
+	    fi
+
+	    # If it's not client-only argument,
+	    # place the argument in a numbered file within our temp
 	    # directory.
 	    # o We don't write a newline at the end, since newline could be
 	    #   part of the argument.
@@ -234,20 +244,16 @@ function parse_options {
 	    #   in order to avoid having 'echo' interpret the output as
 	    #   its own option. We then remove the X.
 	    #   There must be a better way.
-	    echo -n "X$1" > "$tmpdir_client/argv$argc"
-	    sed -i "s|^X||" "$tmpdir_client/argv$argc"
-
-	    # Does the final argument file contain client-side data
-	    # which must be changed to server-side data?
-	    if test "X$arg_subst" != "X" -a $advance = 1; then
-		sed -i "s|$stap_arg|$arg_subst|" "$tmpdir_client/argv$argc"
-		arg_subst=
+	    if test $client_arg = 0; then
+		echo -n "X$arg" > "$tmpdir_client/argv$argc"
+		sed -i "s|^X||" "$tmpdir_client/argv$argc"
+		argc=$(($argc + 1))
 	    fi
 
 	    # Get the next argument.
 	    shift
-	    argc=$(($argc + 1))
 	    advance=$(($advance - 1))
+	    packed_options='-'
 	done
     done
 
@@ -256,7 +262,8 @@ function parse_options {
     if test "X$script_file" != "X"; then
 	local local_name
 	if test "$script_file" != "-"; then
-	    local_name=`generate_client_temp_name "$script_file"`
+	    generate_client_temp_name "$script_file"
+	    local_name="$client_temp_name"
 	else
 	    local_name="-"
 	fi
@@ -277,8 +284,9 @@ function parse_options {
 # Collect an argument to the given option
 function get_arg {
     # Remove first character.
-    local opt=`expr "$1" : '$.$.*'`
-    local first=`expr "$1" : '.$.*$'`
+    local opt="${1:0:1}"
+    local first="${1:1}"
+    packed_options="${packed_options}$opt"
 
     # Advance to the next token, if the first one is exhausted.
     if test "X$first" = "X"; then
@@ -294,7 +302,8 @@ function get_arg {
 #
 # Process the --ssl option.
 function process_ssl {
-    local db=`expr "$1" : '--ssl=$.*$'`
+    client_arg=1
+    local db="${1:6}"
 
     test "X$db" != "X" || \
 	fatal "Missing argument to --ssl"
@@ -308,7 +317,8 @@ function process_ssl {
 #
 # Process the --server option.
 function process_server {
-    local spec=`expr "$1" : '--server=$.*$'`
+    client_arg=1
+    local spec="${1:9}"
 
     test "X$spec" != "X" || \
 	fatal "Missing argument to --server"
@@ -335,12 +345,19 @@ function process_e {
     fi
 }
 
-# function: process_I ARGUMENT
+# function: process_I ARGUMENT ORIGINAL_ARGUMENT
 #
 # Process the -I flag.
 function process_I {
     test "X$1" = "X" && return
-    arg_subst=tapsets/`include_file_or_directory tapsets "$1"`
+    test "${1:0:1}" = "
+" && return
+    include_file_or_directory tapsets "$1"
+    if test $advance = 1; then
+	arg_subst="${packed_options}tapsets/$included_name"
+    else
+	arg_subst="tapsets/$included_name"
+    fi
 }
 
 # function: process_m ARGUMENT
@@ -369,7 +386,7 @@ function process_p {
 #
 # Process the -r flag.
 function process_r {
-    local first_char=`expr "$1" : '$.$.*'`
+    local first_char="${1:0:1}"
 
     if test "$first_char" = "/"; then # fully specified path
         kernel_build_tree="$1"
@@ -402,23 +419,34 @@ function process_a {
     fi
 }
 
-# function: process_R ARGUMENT
+# function: process_R ARGUMENT ORIGINAL_ARGUMENT
 #
 # Process the -R flag.
 function process_R {
     test "X$1" = "X" && return
-    arg_subst=runtime/`include_file_or_directory runtime "$1"`
+    test "${1:0:1}" = "
+" && return
+    include_file_or_directory runtime "$1"
+    if test $advance = 1; then
+	arg_subst="${packed_options}runtime/$included_name"
+    else
+	arg_subst="runtime/$included_name"
+    fi
 }
 
 # function: include_file_or_directory PREFIX NAME
 #
 # Include the given file or directory in the client's temporary
-# tree to be sent to the server.
+# tree to be sent to the server and save it's name in the variable
+# included_name. We use a global variable instread of echoing the
+# result since the use of `include_file_or_directory` loses a trailing
+# newline.
 function include_file_or_directory {
     # Add a symbolic link of the named file or directory to our temporary
     # directory, but only if the file or directory exists.
-    local local_name=`generate_client_temp_name "$2"`
-    echo "$local_name"
+    generate_client_temp_name "$2"
+    local local_name="$client_temp_name"
+    included_name="$local_name"
     test -e "/$local_name" || return
 
     local local_dirname=`dirname "$local_name"`
@@ -431,15 +459,20 @@ function include_file_or_directory {
 # function: generate_client_temp_name NAME
 #
 # Generate the name to be used for the given file/directory relative to the
-# client's temporary directory.
+# client's temporary directory and stores it in the variable
+# client_temp_name. We use a global variable instread of echoing the
+# result since the use of `generate_client_temp_name` loses a trailing
+# newline.
 function generate_client_temp_name {
     # Transform the name into a fully qualified path name
-    local full_name=`echo "X$1" | sed "s,^X\\$[^/]\\$,$wd/\\\\1," | sed 's,^X,,'`
+    local full_name="$1"
+    test "${full_name:0:1}" != "/" && full_name="$wd/$full_name"
 
     # The same name without the initial / or trailing /
-    local local_name=`echo "$full_name" | sed 's,^/$.*$,\1,'`
-    local_name=`echo "$local_name" | sed 's,$.*$/$,\1,'`
-    echo "$local_name"
+    local local_name="${full_name:1}"
+    test "${local_name: -1:1}" = "/" && local_name="${local_name:0:$((${#local_name}-1))}"
+
+    client_temp_name="$local_name"
 }
 
 # function: create_request
@@ -456,7 +489,7 @@ function create_request {
 		fatal "Cannot create temporary directory " $tmpdir_client/script
 	    cat > "$tmpdir_client/script/$script_file"
 	else
-	    include_file_or_directory script "$script_file" > /dev/null
+	    include_file_or_directory script "$script_file"
 	fi
     fi
 
@@ -505,7 +538,7 @@ function unpack_response {
     # 2) a file called stderr
     # 3) a file called rc
     # 4) optionally a directory named to match stap??????
-    num_files=`ls $tmpdir_server | wc -l`
+    local num_files=`ls $tmpdir_server | wc -l`
     test $num_files = 4 -o $num_files = 3 || \
 	fatal "Wrong number of files in server's temp directory"
     test -f $tmpdir_server/stdout || \
--- a/stap-gen-cert
+++ b/stap-gen-cert
@@ -3,7 +3,7 @@
 # Generate a certificate for the systemtap server and add it to the
 # database of trusted servers for the client.
 #
-# Copyright (C) 2008, 2009 Red Hat Inc.
+# Copyright (C) 2008-2010 Red Hat Inc.
 #
 # This file is part of systemtap, and is free software.  You can
 # redistribute it and/or modify it under the terms of the GNU General
@@ -14,61 +14,68 @@
 . ${PKGLIBEXECDIR}stap-env
 
 # Obtain the certificate database directory name.
-serverdb=$1
+serverdb="$1"
 if  test "X$serverdb" = "X"; then
-    serverdb=$stap_ssl_db/server
+    serverdb="$stap_ssl_db/server"
 fi
-rm -fr $serverdb
+rm -fr "$serverdb"
 
 # Create the server's certificate database directory.
-if ! mkdir -p -m 755 $serverdb; then
+if ! mkdir -p -m 755 "$serverdb"; then
     echo "Unable to create the server certificate database directory: $serverdb" >&2
     exit 1
 fi
 
 # Create the certificate database password file. Care must be taken
 # that this file is only readable by the owner.
-if ! (touch $serverdb/pw && chmod 600 $serverdb/pw); then
+if ! (touch "$serverdb/pw" && chmod 600 "$serverdb/pw"); then
     echo "Unable to create the server certificate database password file: $serverdb/pw" >&2
     exit 1
 fi
 
 # Generate a random password.
-mkpasswd -l 20 > $serverdb/pw 2>/dev/null || \
-apg -a 1 -n 1 -m 20 -x 20 > $serverdb/pw 2>/dev/null || \
-(read -n20 password </dev/urandom; echo "$password" > $serverdb/pw)
+mkpasswd -l 20 > "$serverdb/pw" 2>/dev/null || \
+apg -a 1 -n 1 -m 20 -x 20 > "$serverdb/pw" 2>/dev/null || \
+(read -n20 password </dev/urandom; echo "$password" > "$serverdb/pw")
 
 # Generate the server certificate database
-if ! certutil -N -d $serverdb -f $serverdb/pw > /dev/null; then
+if ! certutil -N -d "$serverdb" -f "$serverdb/pw" > /dev/null; then
     echo "Unable to initialize the server certificate database directory: $serverdb" >&2
     exit 1
 fi
 
 # We need some random noise for generating keys
-dd bs=123 count=1 < /dev/urandom > $serverdb/noise 2> /dev/null
+dd bs=123 count=1 < /dev/urandom > "$serverdb/noise" 2> /dev/null
 
 # Generate a request for the server's certificate.
-certutil -R -d $serverdb -f $serverdb/pw -s "CN=Systemtap Compile Server, OU=Systemtap, O=Red Hat, C=US" -o $serverdb/stap.req -z $serverdb/noise 2> /dev/null
-rm -fr $serverdb/noise
+certutil -R -d "$serverdb" -f "$serverdb/pw" -s "CN=Systemtap Compile Server, OU=Systemtap, O=Red Hat, C=US" \
+         -o "$serverdb/stap.req" -z "$serverdb/noise" 2> /dev/null
+rm -fr "$serverdb/noise"
 
 # Create the certificate file first so that it always has the proper access permissions.
-if ! (touch $serverdb/$stap_certfile && chmod 644 $serverdb/$stap_certfile); then
+if ! (touch "$serverdb/$stap_certfile" && chmod 644 "$serverdb/$stap_certfile"); then
     echo "Unable to create the server certificate file: $serverdb/$stap_certfile" >&2
     exit 1
 fi
 
-# Now generate the actual certificate.
-certutil -C -i $serverdb/stap.req -o $serverdb/$stap_certfile -x -d $serverdb -f $serverdb/pw -5 -8 "$HOSTNAME,localhost" >/dev/null <<-EOF
+# Now generate the actual certificate. Make is valid for 1 year.
+certutil -C -i "$serverdb/stap.req" -o "$serverdb/$stap_certfile" -x -d "$serverdb" \
+         -f "$serverdb/pw" -v 12 -5 -8 "$HOSTNAME,localhost" >/dev/null <<-EOF
 1
 3
 7
 8
 y
 EOF
-rm -fr $serverdb/stap.req
+rm -fr "$serverdb/stap.req"
 
 # Add the certificate to the server's certificate/key database as a trusted peer, ssl server and object signer
-certutil -A -n stap-server -t "PCu,,PCu" -i $serverdb/$stap_certfile -d $serverdb -f $serverdb/pw
+certutil -A -n stap-server -t "PCu,,PCu" -i "$serverdb/$stap_certfile" -d "$serverdb" -f "$serverdb/pw"
 
+# Print some information about the certificate.
 echo "Certificate $serverdb/$stap_certfile created and added to database $serverdb"
+certutil -L -d "$serverdb" -n stap-server | \
+  awk '/Validity|Not After|Not Before/ { print $0 }' | \
+  sed 's/^ */  /'
+
 exit 0
--- a/stap-serverd
+++ b/stap-serverd
@@ -70,12 +70,18 @@ function initialization {
 		    -x `which ${stap_exec_prefix}stap-client 2>/dev/null`; then
 		${stap_exec_prefix}stap-authorize-server-cert $ssl_db/$stap_certfile >> $logfile 2>&1
 	    fi
-	elif ! test -f $stap_ssl_db/client/cert8.db; then
-	    # If the client's database does not exist, then initialize it with our certificate.
-	    # Do this only if the client has been installed.
-	    if test -f `which ${stap_exec_prefix}stap-client` -a \
-                    -x `which ${stap_exec_prefix}stap-client`; then
-		${stap_exec_prefix}stap-authorize-server-cert $ssl_db/$stap_certfile >> $logfile 2>&1
+	else
+	    echo "Certificate found in database $ssl_db" >> $logfile
+	    certutil -L -d "$ssl_db" -n stap-server | \
+		awk '/Validity|Not After|Not Before/ { print $0 }' | \
+		sed 's/^ */  /' >> $logfile
+	    if ! test -f $stap_ssl_db/client/cert8.db; then
+		# If the client's database does not exist, then initialize it with our certificate.
+		# Do this only if the client has been installed.
+		if test -f `which ${stap_exec_prefix}stap-client 2>/dev/null` -a \
+                    -x `which ${stap_exec_prefix}stap-client 2>/dev/null`; then
+		    ${stap_exec_prefix}stap-authorize-server-cert $ssl_db/$stap_certfile >> $logfile 2>&1
+		fi
 	    fi
 	fi
     fi
@@ -343,11 +349,19 @@ function advertise_presence {
 function listen {
     # The stap-server-connect program will listen forever
     # accepting requests.
-    ${stap_pkglibexecdir}stap-server-connect \
-	-p $port -n $nss_cert -d $ssl_db -w $nss_pw \
-	-s "$stap_options" \
-	>> $logfile 2>&1 &
-    wait '%${stap_pkglibexecdir}stap-server-connect' >> $logfile 2>&1
+    # CVE-2009-4273 ... or at least, until resource limits fire
+    while true; do # NB: loop to avoid DoS by deliberate rlimit-induced halt
+        # NB: impose resource limits in case of mischevious data inducing
+        # too much / long computation
+        (ulimit -f 50000 -s 1000 -t 60 -u 20 -v 500000;
+         exec ${stap_pkglibexecdir}stap-server-connect \
+	   -p $port -n $nss_cert -d $ssl_db -w $nss_pw \
+	   -s "$stap_options") &
+        stap_server_connect_pid=$!
+        wait
+        # NB: avoid superfast spinning in case of a ulimit or other failure
+        sleep 1
+    done >> $logfile 2>&1
 }
 
 # function: warning [ MESSAGE ]
@@ -379,8 +393,8 @@ function terminate {
     wait '%avahi-publish-service' >> $logfile 2>&1
 
     # Kill any running 'stap-server-connect' job.
-    kill -s SIGTERM '%${stap_pkglibexecdir}stap-server-connect' >> $logfile 2>&1
-    wait '%${stap_pkglibexecdir}stap-server-connect' >> $logfile 2>&1
+    kill -s SIGTERM $stap_server_connect_pid >> $logfile 2>&1
+    wait $stap_server_connect_pid >> $logfile 2>&1
 
     exit
 }
--- a/testsuite/systemtap.server/client_args.exp
+++ b/testsuite/systemtap.server/client_args.exp
@@ -5,33 +5,34 @@ set test "Invalid Server Client Argument
 set test_file $srcdir/systemtap.server/test.stp
 
 # Test invalid combinations.
-set error_regexp ".*You can't specify .* when --unprivileged is specified.*"
+set error_regexp ".*(ERROR)|(You can't specify .* when --unprivileged is specified).*"
 
 set invalid_options [list \
-  "--unprivileged --client-options -a i386" \
   "--unprivileged --client-options -B X=Y" \
   "--unprivileged --client-options -D X=Y" \
   "--unprivileged --client-options -I /tmp" \
   "--unprivileged --client-options -m test" \
   "--unprivileged --client-options -R /tmp" \
-  "--unprivileged --client-options -r [exec uname -r]" \
-  "--unprivileged --client-options -a i386 -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \
-  "--client-options --unprivileged -a i386" \
+  "--unprivileged --client-options -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \
   "--client-options --unprivileged -B X=Y" \
   "--client-options --unprivileged -D X=Y" \
   "--client-options --unprivileged -I /tmp" \
   "--client-options --unprivileged -m test" \
   "--client-options --unprivileged -R /tmp" \
-  "--client-options --unprivileged -r [exec uname -r]" \
-  "--client-options --unprivileged -a i386 -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \
-  "--client-options -a i386 --unprivileged" \
+  "--client-options --unprivileged -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \
   "--client-options -B X=Y --unprivileged" \
   "--client-options -D X=Y --unprivileged" \
   "--client-options -I /tmp --unprivileged" \
   "--client-options -m test --unprivileged" \
   "--client-options -R /tmp --unprivileged" \
-  "--client-options -r [exec uname -r] --unprivileged" \
-  "--client-options -a i386 -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r] --unprivileged" \
+  "--client-options -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r] --unprivileged" \
+  "--client-options -R /path" \
+  "-D \"foo;bar\"" \
+  "-D 2=4" \
+  "-D \"foo;bar\"" \
+  "--client-options -r /path" \
+  "-S /path" \
+  "--client-options -q" \
 ]
 
 foreach options $invalid_options {
@@ -66,9 +67,8 @@ set valid_options [list \
   "-D X=Y" \
   "-I /tmp" \
   "-m test" \
-  "-R /tmp" \
   "-r [exec uname -r]" \
-  "-a i386 -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \
+  "-a i386 -B X=Y -D X=Y -I /tmp -m test -r [exec uname -r]" \
   "--unprivileged" \
   "--unprivileged -a i386" \
   "--unprivileged -B X=Y" \
@@ -80,13 +80,11 @@ set valid_options [list \
   "--unprivileged -a i386 -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \
   "--client-options" \
   "--client-options -a i386" \
-  "--client-options -B X=Y" \
   "--client-options -D X=Y" \
   "--client-options -I /tmp" \
   "--client-options -m test" \
-  "--client-options -R /tmp" \
   "--client-options -r [exec uname -r]" \
-  "--client-options -a i386 -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \
+  "--client-options -a i386 -D X=Y -I /tmp -m test -r [exec uname -r]" \
   "--unprivileged --client-options" \
   "--client-options --unprivileged" \
   "--unprivileged -a i386 --client-options" \
--- a/testsuite/systemtap.server/hello.stp
+++ b/testsuite/systemtap.server/hello.stp
@@ -1,4 +1,4 @@
-#! stap 
+#! stap -p5
 
 probe begin
 {
--- a/testsuite/systemtap.server/server_args.exp
+++ b/testsuite/systemtap.server/server_args.exp
@@ -40,11 +40,13 @@ proc stap_direct_and_with_client {stap s
 	# Some messages contain the names of files or directories
 	# and will be prefixed for the client.
 	if {[regexp "^              (.*)" $expected_line match data]} {
-	    if {[regexp "^              tapsets/.*/$data" $line]} {
+	    # Special characters in the regexp need to be quoted.
+	    regsub -all "\[\"\\\\;\*\]" $data {\\\0} data
+	    if {[regexp "^              tapsets.*/$data" $line]} {
 		incr n
 		continue
 	    }
-	    if {[regexp "^              runtime/.*/$data" $line]} {
+	    if {[regexp "^              runtime.*/$data" $line]} {
 		incr n
 		continue
 	    }
@@ -57,7 +59,9 @@ proc stap_direct_and_with_client {stap s
 	    }
 	} else {
 	    if {[regexp "^Input file '(.*)' is empty or missing." $expected_line match data]} {
-		if {[regexp "^Input file 'script/.*/$data' is empty or missing." $line]} {
+		# Special characters in the regexp need to be quoted.
+		regsub -all "\[\"\\\\;\*\]" $data {\\\0} data
+		if {[regexp "^Input file 'script.*/$data' is empty or missing." $line]} {
 		    incr n
 		    continue
 		}
@@ -110,21 +114,27 @@ if {[installtest_p]} then {
 # for debugging a currently failing case and helps to ensure that previously
 # fixed cases do not regress.
 set previously_fixed [list \
+  "-p1 -I=\\w94\nbh -e -Dhfuo0iu7 -c" \
+  "-p1 -I8o\\2ie -e'1\\ -D\n\" -c" \
+  "-p1 -Ira\\3;c g -e0fle'qq -Dr/316k\\o8 -cjyoc\n3" \
+  "-p1 -I6p3 -elc -Dqgsgv' -c" \
+  "-p1 -I\"vyv;z -ej\"/3 -D/ 01qck\n -c3u55zut" \
+  "-p1 -I1 -eo9e\nx047q -D9xyefk0a -cvl98/x1'i" \
   "-p1 -c; test.stp" \
-  "-p1 -I4hgy96 -R -e5oo39p -Bile\\vp -Ddx8v -c4;" \
-  "-p1 -I -Repwd9 -esq3wors -Btmk;\\t -Dz -c*eibz8h2e" \
-  "-p1 -I -Ry a -em339db5 -B;ae41428d -Du2;c0ps -ch9o\\" \
-  "-p1 -Ipfjps4 -Rx479 -ebug4dc -Bih;fe2 -Du8vd fvkl -c" \
-  "-p1 -I0\"nspzjyf -R -e5r3up8h -Bgqnyjq6w -Dmi;ojp9m -cx;a2fat" \
-  "-p1 -Iu -R9 -ek7;r -Big -Dcu\"; -c\"hc" \
-  "-p1 -Icd4fidq  -Rkj m40mv -edn -B7ria -D;8ha\\cjr -c1*vnq" \
-  "-p1 -I;3 -R3lq;vp -er8e -Bgdqjqdy -D -cb6k29z" \
-  "-p1 -Ircj -R -e -B -D -c\\vmww" \
-  "-p1 -Illc5 -Rug*\\o -e65wof9 -B qr*=x7x5 -D -cgx;" \
-  "-p1 -Iyaj420=3 -R -e\" -Bx68j -D -cd'5mi" \
-  "-p1 -Ir -Rwd8;;sjl -e -Bxh; -D29\\ -cj2szt;4" \
-  "-p1 -Ibno3=b4sk -R*5 -e' -Byl63flos -Dg2-j;e -c2ijx'" \
-  "-p1 -I285v7pl -R9a -eo5\\0 -Bfs* -D86s -c-c*v" \
+  "-p1 -I4hgy96 -e5oo39p -Ddx8v -c4;" \
+  "-p1 -I -esq3wors -Dz -c*eibz8h2e" \
+  "-p1 -I a -em339db5 -Du2;c0ps -ch9o\\" \
+  "-p1 -Ipfjps4 -ebug4dc -Du8vd fvkl -c" \
+  "-p1 -I0\"nspzjyf -e5r3up8h -Dmi;ojp9m -cx;a2fat" \
+  "-p1 -Iu -ek7;r -Dcu\"; -c\"hc" \
+  "-p1 -Icd4fidq  m40mv -edn -D;8ha\\cjr -c1*vnq" \
+  "-p1 -I;3 -er8e -D -cb6k29z" \
+  "-p1 -Ircj -e -D -c\\vmww" \
+  "-p1 -Illc5 -e65wof9 qr*=x7x5 -D -cgx;" \
+  "-p1 -Iyaj420=3 -e\" -D -cd'5mi" \
+  "-p1 -Ir -e -D29\\ -cj2szt;4" \
+  "-p1 -Ibno3=b4sk -e' -Dg2-j;e -c2ijx'" \
+  "-p1 -I285v7pl -eo5\\0 -D86s -c-c*v" \
 ]
 
 set i 0
@@ -140,7 +150,10 @@ foreach options $previously_fixed {
 # Generate semi-random arguments containing with potential problem characters.
 # Check that running systemtap with the client/server generates output
 # comparable to running stap directly.
-set dangerous_options [list "-I" "-R" "-e" "-B" "-D" "-c"]
+set dangerous_options [list "-I" "-e" "-D" "-c" "-S"]
+# NB: Other options could be candidates here, like -r and -B, but
+# there stap-server imposes more restrictions than local stap, so
+# this simple error-matching test cannot use them.
 set argchars "0123456789/;*'=-\\\"\n abcdefghijklmnopqrstuvwxyz"
 
 for {set i 0} {$i < $iterations} {incr i} {
--- a/util.cxx
+++ b/util.cxx
@@ -1,5 +1,5 @@
 // Copyright (C) Andrew Tridgell 2002 (original file)
-// Copyright (C) 2006, 2009 Red Hat Inc. (systemtap changes)
+// Copyright (C) 2006-2010 Red Hat Inc. (systemtap changes)
 //
 // This program is free software; you can redistribute it and/or
 // modify it under the terms of the GNU General Public License as
@@ -19,6 +19,8 @@
 #include "sys/sdt.h"
 #include <stdexcept>
 #include <cerrno>
+#include <map>
+#include <string>
 
 extern "C" {
 #include <fcntl.h>
@@ -31,6 +33,7 @@ extern "C" {
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <unistd.h>
+#include <regex.h>
 }
 
 using namespace std;
@@ -413,4 +416,35 @@ kill_stap_spawn(int sig)
   return spawned_pid ? kill(spawned_pid, sig) : 0;
 }
 
+
+void assert_regexp_match (const string& name, const string& value, const string& re)
+{
+  typedef map<string,regex_t*> cache;
+  static cache compiled;
+  cache::iterator it = compiled.find (re);
+  regex_t* r = 0;
+  if (it == compiled.end())
+    {
+      r = new regex_t;
+      int rc = regcomp (r, re.c_str(), REG_ICASE|REG_NOSUB|REG_EXTENDED);
+      if (rc) {
+        cerr << "regcomp " << re << " (" << name << ") error rc=" << rc << endl;
+        exit(1);
+      }
+      compiled[re] = r;
+    }
+  else
+    r = it->second;
+
+  // run regexec
+  int rc = regexec (r, value.c_str(), 0, 0, 0);
+  if (rc)
+    {
+      cerr << "ERROR: Safety pattern mismatch for " << name
+           << " ('" << value << "' vs. '" << re << "') rc=" << rc << endl;
+      exit(1);
+    }
+}
+
+
 /* vim: set sw=2 ts=8 cino=>4,n-2,{2,^-2,t0,(0,u0,w1,M1 : */
--- a/util.h
+++ b/util.h
@@ -21,7 +21,7 @@ const std::string cmdstr_quoted(const st
 std::string git_revision(const std::string& path);
 int stap_system(int verbose, const std::string& command);
 int kill_stap_spawn(int sig);
-
+void assert_regexp_match (const std::string& name, const std::string& value, const std::string& re);
 
 // stringification generics

Places

File systemtap-CVE-2009-4273.diff of Package systemtap (Revision 29904bf9b6078543678b15b68c2117ef)

Places