Backport #7325

Marshal#load taints classes if they are referenced in a marsheled object

Added by Uriel Katz over 1 year ago. Updated over 1 year ago.

[ruby-core:49198]
Status:Closed
Priority:Normal
Assignee:Usaku NAKAMURA

Description

=begin
= Reproducing steps:
ruby taint.rb

= Output of this script in my computer running 1.9.3-p327:
Before marshal is tainted?: false
After marshal is tainted?: true
Safe level when calling tainted method using call: 4
Safe level when calling tainted method directly: 0

= Expected:
MyObject#test shouldn't be tainted as it was defined in my own source and what was saved into the file is just a reference to MyObject class ("\u0004\bc\rMyObject")

= Actual:
MyObject#test is tainted and calling it using Method#call will make it run in safe-level 4.

= Some background on how I got to this issue:
I wrote some RPC code that accepts a class and method name and does the invocation,the way I call the method is getting the method from the instance using something like: "clsinstance.method(methodname).call"

I used Rails.cache with FileStore (which uses Marshal#load from file) to cache a object that had references to classes.

After reading from the cache all other requests saw the classes as tainted and when calling the methods they ran at $SAFE=4 which caused it to fail (even puts doesn't work at that level :)

This issue also made me understand that there is 2 potential bugs in Rails.
=end

taint.rb Magnifier (458 Bytes) Uriel Katz, 11/10/2012 10:00 PM

Associated revisions

Revision 38468
Added by Usaku NAKAMURA over 1 year ago

merge revision(s) 38357,38363: [Backport #7325]

* marshal.c (r_entry0): don't taint classes and modules because
  Marshal.load just return the dumped classes and modules.
  [Bug #7325] 

* test/ruby/test_marshal.rb: related test.
  Marshal.load just returns the dumped classes and modules.

History

#1 Updated by Yusuke Endoh over 1 year ago

  • Status changed from Open to Assigned
  • Assignee set to Shugo Maeda
  • Target version set to 2.0.0

Summarized:

p Integer.tainted? #=> false
Marshal.load(Marshal.dump(Integer).taint)
p Integer.tainted? #=> expected: false, actual: true

Indeed, it looks weird. Shugo-san, what do you think?

Yusuke Endoh mame@tsg.ne.jp

#2 Updated by Shugo Maeda over 1 year ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r38357.
Uriel, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • marshal.c (r_entry0): don't taint classes and modules because
    Marshal.load just return the dumped classes and modules.
    [Bug #7325]

  • test/ruby/test_marshal.rb: related test.

#3 Updated by Usaku NAKAMURA over 1 year ago

  • Tracker changed from Bug to Backport
  • Project changed from ruby-trunk to Backport93
  • Status changed from Closed to Assigned
  • Assignee changed from Shugo Maeda to Usaku NAKAMURA
  • Target version deleted (2.0.0)

#4 Updated by Usaku NAKAMURA over 1 year ago

memo: r38357 is also related.

#5 Updated by Usaku NAKAMURA over 1 year ago

  • Status changed from Assigned to Closed

This issue was solved with changeset r38468.
Uriel, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


merge revision(s) 38357,38363: [Backport #7325]

* marshal.c (r_entry0): don't taint classes and modules because
  Marshal.load just return the dumped classes and modules.
  [Bug #7325] 

* test/ruby/test_marshal.rb: related test.
  Marshal.load just returns the dumped classes and modules.

Also available in: Atom PDF