1.8.4 untaint behavior


#1

Hi,

I have a script running at $SAFE = 1, and after upgrading ruby
from 1.8.2 to 1.8.4 (ruby 1.8.4 (2005-12-24) [i686-linux]),
I started seeing SecurityError exceptions.

By adding printouts to the following code, I verified that
line.untaint is occurring, and line.tainted? is false, but
after the regexp match on line, the addr, port, and nick
fields that come out of $1 $2 $3 are all tainted:

def read_servers_list(filename)
servers = []
IO.foreach($servers_list_filename) do |line|
line.chop!
line.strip!
next if line.empty? || line =~ /\A\s*#/
if line =~ /\A[A-Za-z0-9\s.:-]+\z/
line.untaint
else
logerr(“read_servers_list: refusing to untaint #{line.inspect}
because of unexpected characters”)
next
end
if line =~ /(\w+)\s+([^\s:]+)(?::(\d+))?/
addr, port, nick = $2, $3 || “27910”, $1
#
##
###
### At this point,
### line.tainted? => false
### [addr,port,nick].all? {|o| o.tainted?} => true
###
##
#
servers << ServerAddr.new(addr, port, nick)
else
logerr(“read_servers_list: couldn’t parse #{line.inspect}”)
end
end
servers
end

Is this a bug? Not sure why values extracted from an
untainted object would be re-tainted. (Or have I made some
crass mistake? :slight_smile:

Thanks for your help,

Regards,

Bill


#2

Hi,

I’ve reduced it to 10 lines or so, with no external I/O.
I think this is a bug, unless I’m really doing something dumb.

#!/usr/bin/env ruby

$SAFE = 1 if $SAFE < 1

line = "vanilla tastyspleen.net:27912\n"
line.taint
line.chop!
line.strip!
line =~ /\A[A-Za-z0-9\s.:-]+\z/   #### this triggers it
line.untaint
if line =~ /(\w+)\s+([^\s:]+)(?::(\d+))?/
  addr, port, nick = $2, $3 || "27910", $1
  $stderr.puts "a=#{addr.tainted?} p=#{port.tainted?} n=#{nick.tainted?} 
l=#{line.tainted?} #{line}"
end

For me, (ruby 1.8.4 (2005-12-24) [i686-linux]), the above prints:

a=true p=true n=true l=false vanilla tastyspleen.net:27912

indicating line is not tainted, but the capture values pulled out of it
are.

But, if I remove the “this triggers it” line, then the program prints:

a=false p=false n=false l=false vanilla tastyspleen.net:27912

which is how it used to behave under 1.8.2.

Here’s a one-liner version of it:

ruby -ve ‘$SAFE=1; l=“foo”.taint; l=~/(.)/; l.untaint; l=~/(.)/; p
$1.tainted?’
ruby 1.8.4 (2005-12-24) [i686-linux]
true

ruby -ve ‘$SAFE=1; l=“foo”.taint; l.untaint; l=~/(.)/; p
$1.tainted?’
ruby 1.8.4 (2005-12-24) [i686-linux]
false

Hope this helps,

Regards,

Bill


#3

Hi,

At Wed, 18 Jan 2006 09:11:09 +0900,
Bill K. wrote in [ruby-talk:176063]:

I’ve reduced it to 10 lines or so, with no external I/O.
I think this is a bug, unless I’m really doing something dumb.

Probably.

The reason is underlying shared string object still keeps
tainted after the original string got untainted.

This code shows a similar problem.

line = “x” * 100
line.taint
line[0…-1]
line.untaint
p line.tainted?, line[0…-1].tainted?

Index: array.c

RCS file: /cvs/ruby/src/ruby/array.c,v
retrieving revision 1.186
diff -U2 -p -r1.186 array.c
— array.c 12 Dec 2005 16:46:59 -0000 1.186
+++ array.c 18 Jan 2006 02:12:09 -0000
@@ -73,4 +73,12 @@ rb_ary_freeze(VALUE ary)
}

+/* :nodoc: */
+VALUE
+rb_ary_untaint(VALUE ary)
+{

  • rb_ary_modify(ary);
  • return rb_obj_untaint(ary);
    +}

/*

  • call-seq:
    @@ -2852,4 +2860,5 @@ Init_Array(void)
    rb_define_method(rb_cArray, “to_ary”, rb_ary_to_ary_m, 0);
    rb_define_method(rb_cArray, “frozen?”, rb_ary_frozen_p, 0);
  • rb_define_method(rb_cArray, “untaint”, rb_ary_untaint, 0);

    rb_define_method(rb_cArray, “==”, rb_ary_equal, 1);
    Index: string.c
    ===================================================================
    RCS file: /cvs/ruby/src/ruby/string.c,v
    retrieving revision 1.242
    diff -U2 -p -r1.242 string.c
    — string.c 29 Dec 2005 17:03:27 -0000 1.242
    +++ string.c 18 Jan 2006 02:12:46 -0000
    @@ -565,4 +565,12 @@ rb_str_freeze(VALUE str)
    }

+/* :nodoc: */
+VALUE
+rb_str_untaint(VALUE str)
+{

  • rb_str_modify(str);
  • return rb_obj_untaint(str);
    +}

VALUE
rb_str_dup_frozen(VALUE str)
@@ -4190,4 +4198,5 @@ Init_String(void)
rb_define_method(rb_cString, “initialize”, rb_str_init, -1);
rb_define_method(rb_cString, “initialize_copy”, rb_str_replace, 1);

  • rb_define_method(rb_cString, “untaint”, rb_str_untaint, 0);
    rb_define_method(rb_cString, “<=>”, rb_str_cmp_m, 1);
    rb_define_method(rb_cString, “==”, rb_str_equal, 1);

#4

Ni Nobu,

From: “nobuyoshi nakada” removed_email_address@domain.invalid

At Wed, 18 Jan 2006 09:11:09 +0900,
Bill K. wrote in [ruby-talk:176063]:

I’ve reduced it to 10 lines or so, with no external I/O.
I think this is a bug, unless I’m really doing something dumb.

Probably.

The reason is underlying shared string object still keeps
tainted after the original string got untainted.

[patch elided]

Awesome, thanks !!

Regards,

Bill