Bug #7759

Marshal.load is not documented to be dangerous

Added by Charlie Somerville about 1 year ago. Updated about 1 year ago.

[ruby-core:51765]
Status:Closed
Priority:Normal
Assignee:-
Category:doc
Target version:2.0.0
ruby -v:ruby 2.0.0dev (2013-01-07 trunk 38733) [x86_64-darwin12.2.1] Backport:

Description

=begin
Marshal.load is incredibly powerful, and also incredibly dangerous.

Unfortunately, many developers use it inappropriately and unmarshal user input. This can lead to a wide range of vulnerabilities, including remote code execution.

Marshal.load should be documented as dangerous and the documentation should also mention that it should only be used on trusted data.
=end


Related issues

Related to ruby-trunk - Bug #7780: Marshal & YAML should deserialize only basic types by def... Assigned 02/04/2013

Associated revisions

Revision 38999
Added by Charlie Somerville about 1 year ago

marshal.c: warn against using Marshal.load on untrusted data

  • marshal.c (marshal_load): Add documentation warning against using Marshal.load on untrusted data [Bug #7759]

Revision 39005
Added by Charlie Somerville about 1 year ago

  • marshal.c: add security considerations to marshal overview, refer to overview from Marshal.load documentation [#7759]

History

#1 Updated by Motohiro KOSAKI about 1 year ago

Marshal.load is incredibly powerful, and also incredibly dangerous.

Unfortunately, many developers use it inappropriately and unmarshal user input. This can lead to a wide range of vulnerabilities, including remote code execution.

Marshal.load should be documented as dangerous and the documentation should also mention that it should only be used on trusted data.

Makes sense. Can you please consider to write down the explanation?

#2 Updated by Nobuyoshi Nakada about 1 year ago

charliesome (Charlie Somerville) wrote:

Unfortunately, many developers use it inappropriately and unmarshal user input. This can lead to a wide range of vulnerabilities, including remote code execution.

Can't you elaborate it, probably, at security@ruby-lang.org?

Marshal.load should be documented as dangerous and the documentation should also mention that it should only be used on trusted data.

I've thought it's a common sense, isn't it?

#3 Updated by Charlie Somerville about 1 year ago

I've thought it's a common sense, isn't it?

You would imagine so, however I have seen a lot of code that does unmarshal untrusted data.

I will send an example to security@ruby-lang.org. Please note that I do not consider this a vulnerability in Ruby. Marshal is dangerous by design. This is an education problem - we need to document the fact that it is dangerous.

#4 Updated by Koichi Sasada about 1 year ago

(2013/01/31 8:59), charliesome (Charlie Somerville) wrote:

Unfortunately, many developers use it inappropriately and unmarshal user input. This can lead to a wide range of vulnerabilities, including remote code execution.

Could you explain attack scenario?

--
// SASADA Koichi at atdot dot net

#5 Updated by Charlie Somerville about 1 year ago

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

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


marshal.c: warn against using Marshal.load on untrusted data

  • marshal.c (marshal_load): Add documentation warning against using Marshal.load on untrusted data [Bug #7759]

Also available in: Atom PDF