1.8.4 untaint behavior

ruby-talk
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 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);

--
Nobu Nakada

4

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

···

From: "nobuyoshi nakada" <nobuyoshi.nakada@ge.com>