1.8.4 untaint behavior

[Bill Kelly]

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?


Thanks for your help,

Regards,

Bill

 

[Bill Kelly]

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

 

[nobuyoshi nakada]

Hi,

At Wed, 18 Jan 2006 09:11:09 +0900,
Bill Kelly 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);

 

[Bill Kelly]

Ni Nobu,

At Wed, 18 Jan 2006 09:11:09 +0900,
Bill Kelly 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