Note: Not including testsuite part. commit c0d1b5a004b9949bb455b7dbe17b335b7cab9ead Author: Frank Ch. Eigler 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. diff --git a/main.cxx b/main.cxx index 8f5ee72..2dba179 100644 --- 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; } @@ -708,12 +708,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; @@ -722,6 +722,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(); @@ -766,15 +767,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': @@ -821,16 +821,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; @@ -861,9 +864,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: diff --git a/stap-serverd b/stap-serverd index eda9711..5820286 100755 --- a/stap-serverd +++ b/stap-serverd @@ -360,11 +360,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 ] @@ -396,8 +404,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 } diff --git a/util.cxx b/util.cxx index 736e5a3..73ba167 100644 --- 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 #include +#include +#include extern "C" { #include @@ -31,6 +33,7 @@ extern "C" { #include #include #include +#include } 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 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 : */ diff --git a/util.h b/util.h index 8fc64cb..75e198c 100644 --- a/util.h +++ b/util.h @@ -21,7 +21,7 @@ const std::string cmdstr_quoted(const std::string& cmd); 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 commit cc9e5488d82b728e568bca1f8d6094856fc8e641 Author: Frank Ch. Eigler Date: Fri Feb 12 10:39:58 2010 -0500 PR11105 part 2a, fix buggy \\. in -r option regexp diff --git a/main.cxx b/main.cxx index 2dba179..b5fdbc0 100644 --- a/main.cxx +++ b/main.cxx @@ -768,7 +768,7 @@ main (int argc, char * const argv []) case 'r': if (client_options) // NB: no paths! - assert_regexp_match("-r parameter from client", optarg, "^[a-z0-9_\\.-]+$"); + assert_regexp_match("-r parameter from client", optarg, "^[a-z0-9_.-]+$"); setup_kernel_release(s, optarg); break; commit c8408b459b88a5aa5f4325e690aef95b5da7c2eb Author: Mark Wielaard Date: Sun Feb 14 21:42:06 2010 +0100 PR11281 Allow negative values for -D argument. Change regexp match to "^[a-z_][a-z_0-9]*(=-?[a-z_0-9]+)?$". * main.cxx (main): case 'D' allow optional single minus sign after equal in assert_regexp_match(). diff --git a/main.cxx b/main.cxx index b5fdbc0..faac7f8 100644 --- a/main.cxx +++ b/main.cxx @@ -821,7 +821,7 @@ 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]+)?$"); + 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));