[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 10 May 2011 17:05:11 -0400
From: William Cohen <wcohen@...hat.com>
To: Huzaifa Sidhpurwala <huzaifas@...hat.com>
CC: oss-security@...ts.openwall.com, Jan Lieskovsky <jlieskov@...hat.com>,
"Steven M. Christey" <coley@...us.mitre.org>,
Stephane Chauveau <stephane.chauveau@...s-entreprise.com>,
Maynard Johnson <maynardj@...ibm.com>,
Robert Richter <robert.richter@....com>
Subject: Re: Re: CVE Request -- oprofile -- Local privilege
escalation via crafted opcontrol event parameter when authorized by sudo
On 05/03/2011 05:36 AM, Huzaifa Sidhpurwala wrote:
> Hi William,
> On 05/01/2011 07:30 AM, William Cohen wrote:
>>
>> I don't know if this is the best way to fix this issue, but attached is a patch that filters out all but alpha numeric characters and '_'. Feedback on the patch would be appreciated.
>>
>
> It appears from the debian bug, that there may be others way to exploit
> this issue as well. hence i think we need a revised patch?
>
>
>
The patches mentioned in the previous email.
-Will
>From d52d142365cedb6c11e0d57835a6530cb9687474 Mon Sep 17 00:00:00 2001
From: William Cohen <wcohen@...hat.com>
Date: Tue, 10 May 2011 11:44:11 -0400
Subject: [PATCH 1/4] Sanitize Event Names
The event names need to be sanitized to only allow alphanumeric characters to
address CVE-2011-1760.
---
utils/opcontrol | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/utils/opcontrol b/utils/opcontrol
index 3a8a814..f2558e6 100644
--- a/utils/opcontrol
+++ b/utils/opcontrol
@@ -390,7 +390,17 @@ get_event()
set_event()
{
- eval "CHOSEN_EVENTS_$1=$2"
+ clean1="`echo "${1}" | tr -cd '[:alnum:]_'`"
+ clean2="`echo "${2}" | tr -cd '[:alnum:]_:'`"
+ if [ "x$1" != "x$clean1" ]; then
+ echo "Invalid event number \"$1\"."
+ exit 1
+ fi
+ if [ "x$2" != "x$clean2" ]; then
+ echo "Invalid event \"$2\"."
+ exit 1
+ fi
+ eval "CHOSEN_EVENTS_$clean1=$clean2"
}
--
1.7.1
>From b38fae2cd24feee3e979c5f5eada8549cf2e39b2 Mon Sep 17 00:00:00 2001
From: William Cohen <wcohen@...hat.com>
Date: Tue, 10 May 2011 11:50:33 -0400
Subject: [PATCH 2/4] Ensure that --save only saves things in $SESSION_DIR
It was possible use the --session-dir and --save to copy files into arbitrary
locations. This change limits the --save to moving directories within
the $SESSION_DIR.
---
utils/opcontrol | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/utils/opcontrol b/utils/opcontrol
index f2558e6..d25b386 100644
--- a/utils/opcontrol
+++ b/utils/opcontrol
@@ -75,6 +75,16 @@ error_if_not_number()
fi
}
+# check value is a base filename
+error_if_not_basename()
+{
+ bname=`basename "$2"`
+ if [[ "x$2" != "x$bname" ]] ; then
+ echo "Argument for $1, $2, is not a base filename." >&2
+ exit 1
+ fi
+}
+
# rm_device arguments $1=file_name
rm_device()
{
@@ -764,6 +774,7 @@ do_options()
--save)
error_if_empty $arg $val
+ error_if_not_basename $arg $val
DUMP=yes
SAVE_SESSION=yes
SAVE_NAME=$val
--
1.7.1
>From 02220ca51a25913a5b81885066ac4ff2ca2490c5 Mon Sep 17 00:00:00 2001
From: William Cohen <wcohen@...hat.com>
Date: Tue, 10 May 2011 14:38:26 -0400
Subject: [PATCH 3/4] Avoid blindly source $SETUP_FILE with '.'
If there could be arbitrary commands in the $SETUP_FILE, the '.' command
would blindly execute them. This change limits do_load_setup to only
assigning values to variables.
---
utils/opcontrol | 17 +++++++++++++----
1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/utils/opcontrol b/utils/opcontrol
index d25b386..7b82b99 100644
--- a/utils/opcontrol
+++ b/utils/opcontrol
@@ -469,9 +469,19 @@ do_load_setup()
{
if test -f "$SETUP_FILE"; then
# load the actual information from file
- # FIXME this is insecure, arbitrary commands could be added to
- # $SETUP_FILE and be executed as root
- . $SETUP_FILE
+ while IFS== read -r arg val; do
+ clean_arg="`echo "${arg}" | tr -cd '[:alnum:]_'`"
+ clean_val="`echo "${val}" | tr -cd '[:alnum:]_:/.-'`"
+ if [ "x$arg" != "x$clean_arg" ]; then
+ echo "Invalid variable \"$arg\" in $SETUP_FILE."
+ exit 1
+ fi
+ if [ "x$val" != "x$clean_val" ]; then
+ echo "Invalid value \"$val\" in $SETUP_FILE."
+ exit 1
+ fi
+ eval "${clean_arg}=${clean_val}"
+ done < $SETUP_FILE
fi
}
@@ -774,7 +784,6 @@ do_options()
--save)
error_if_empty $arg $val
- error_if_not_basename $arg $val
DUMP=yes
SAVE_SESSION=yes
SAVE_NAME=$val
--
1.7.1
>From e40f18454d0fbae93812fa25c78fabec58270a67 Mon Sep 17 00:00:00 2001
From: William Cohen <wcohen@...hat.com>
Date: Tue, 10 May 2011 16:42:31 -0400
Subject: [PATCH 4/4] Do additional checks on user supplied arguments
Avoid blindly setting variable to user-supplied values. Check to the values
to make sure they do not contain odd punctuation to address CVE-2011-1760.
---
utils/opcontrol | 36 ++++++++++++++++++++++--------------
1 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/utils/opcontrol b/utils/opcontrol
index 7b82b99..6cc0198 100644
--- a/utils/opcontrol
+++ b/utils/opcontrol
@@ -68,6 +68,7 @@ guess_number_base()
# check value is a valid number
error_if_not_number()
{
+ error_if_empty $1 $2
guess_number_base $2
if test "$?" -eq 0 ; then
echo "Argument for $1, $2, is not a valid number." >&2
@@ -85,6 +86,16 @@ error_if_not_basename()
fi
}
+error_if_invalid_arg()
+{
+ error_if_empty $1 $2
+ clean_val="`echo "$2" | tr -cd '[:alnum:]_:/,\-.'`"
+ if [ "x$2" != "x$clean_val" ]; then
+ echo "Argument for $1, $2, is not valid argument." >&2
+ exit 1
+ fi
+}
+
# rm_device arguments $1=file_name
rm_device()
{
@@ -471,7 +482,7 @@ do_load_setup()
# load the actual information from file
while IFS== read -r arg val; do
clean_arg="`echo "${arg}" | tr -cd '[:alnum:]_'`"
- clean_val="`echo "${val}" | tr -cd '[:alnum:]_:/.-'`"
+ clean_val="`echo "${val}" | tr -cd '[:alnum:]_:/,\-.'`"
if [ "x$arg" != "x$clean_arg" ]; then
echo "Invalid variable \"$arg\" in $SETUP_FILE."
exit 1
@@ -783,7 +794,7 @@ do_options()
;;
--save)
- error_if_empty $arg $val
+ error_if_invalid_arg $arg $val
DUMP=yes
SAVE_SESSION=yes
SAVE_NAME=$val
@@ -808,7 +819,6 @@ do_options()
# already processed
;;
--buffer-size)
- error_if_empty $arg $val
error_if_not_number $arg $val
BUF_SIZE=$val
DO_SETUP=yes
@@ -818,7 +828,6 @@ do_options()
echo "$arg unsupported for this kernel version"
exit 1
fi
- error_if_empty $arg $val
error_if_not_number $arg $val
BUF_WATERSHED=$val
DO_SETUP=yes
@@ -828,13 +837,12 @@ do_options()
echo "$arg unsupported for this kernel version"
exit 1
fi
- error_if_empty $arg $val
error_if_not_number $arg $val
CPU_BUF_SIZE=$val
DO_SETUP=yes
;;
-e|--event)
- error_if_empty $arg $val
+ error_if_invalid_arg $arg $val
# reset any read-in defaults from daemonrc
if test "$SEEN_EVENT" = "0"; then
NR_CHOSEN=0
@@ -855,7 +863,6 @@ do_options()
DO_SETUP=yes
;;
-c|--callgraph)
- error_if_empty $arg $val
if test ! -f $MOUNT/backtrace_depth; then
echo "Call-graph profiling unsupported on this kernel/hardware" >&2
exit 1
@@ -865,7 +872,7 @@ do_options()
DO_SETUP=yes
;;
--vmlinux)
- error_if_empty $arg $val
+ error_if_invalid_arg $arg $val
VMLINUX=$val
DO_SETUP=yes
;;
@@ -874,32 +881,32 @@ do_options()
DO_SETUP=yes
;;
--kernel-range)
- error_if_empty $arg $val
+ error_if_invalid_arg $arg $val
KERNEL_RANGE=$val
DO_SETUP=yes
;;
--xen)
- error_if_empty $arg $val
+ error_if_invalid_arg $arg $val
XENIMAGE=$val
DO_SETUP=yes
;;
--active-domains)
- error_if_empty $arg $val
+ error_if_invalid_arg $arg $val
ACTIVE_DOMAINS=$val
DO_SETUP=yes
;;
--note-table-size)
- error_if_empty $arg $val
if test "$KERNEL_SUPPORT" = "yes"; then
echo "\"$arg\" meaningless on this kernel" >&2
exit 1
else
+ error_if_not_number $arg $val
NOTE_SIZE=$val
fi
DO_SETUP=yes
;;
-i|--image)
- error_if_empty $arg $val
+ error_if_invalid_arg $arg $val
if test "$val" = "all"; then
IMAGE_FILTER=
else
@@ -912,6 +919,7 @@ do_options()
if test -z "$val"; then
VERBOSE="all"
else
+ error_if_invalid_arg $arg $val
VERBOSE=$val
fi
;;
@@ -1845,7 +1853,7 @@ check_options_early()
exit 0
;;
--session-dir)
- error_if_empty $arg $val
+ error_if_invalid_arg $arg $val
SESSION_DIR="$val"
DO_SETUP=yes
# do not exit early
--
1.7.1
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Powered by Openwall GNU/*/Linux -
Powered by OpenVZ
