Mash.load is insecure

[bo-tato]

Using Mash.load on untrusted input is insecure, as it calls ERB.new, which will execute arbitrary ruby code. This might seem obvious as the example in the documentation shows using ERB, but the documentation also says it calls YAML.safe_load and includes permitted_symbols and permitted_classes options intended for controlling the security of loading yaml, so it seems to imply there is some sort of security control. I think Mash.load should be secure by default and only run ERB.new if you explicitly pass an option to use it, or at least the documentation should be updated with a warning so it's more clear that people shouldn't run Mash.load on untrusted input

[dblock]

Yes, this looks like a problem, at least in the documentation. Hopefully people only use it to load YML files, which seems to be the case. I propose three things:

1. Highlight that Mash.load executes ERB.new to resolve templates in the documentation, and should not be run on untrusted code.
2. Split outYamlParser from YamlErbParser that doesn't do ERB.new.
3. Default to YamlParser, however this would be a breaking change, so we'd need a major version bump.

Want to take it on?

[bo-tato]

Hopefully people only use it to load YML files, which seems to be the case.

Cool yea it seems applications are just using it to load their own config files and not on user-provided files. I noticed as I was making a little script for dealing with json responses from an api (yaml.safe_load also parses json), hashie is a nice library that makes it convenient to deal with :). But the api could return stuff with ruby in erb templates and run code on my computer if I used Mash.load, so I used instead Mash.new(JSON.parse(...))

Want to take it on?

I'm about one week in to learning ruby, I work pentesting so I've audited a handful of rails apps and have some experience reading ruby code, just not writing it. It seems like a straightforward enough first task so I'll give it a try, but definitely review my attempt and change anything that's not good ruby style.
Maybe also in the new default plain YamlParser, we could check if there is any erb template present, and if so raise an exception with a message explaining the change and that assuming they are loading their own config file or a file they trust they probably want to call Mash.load with the parser: YamlErbParser option? to try and limit the pain a breaking change will cause

[dblock]

1. I would avoid Mash where possible, https://code.dblock.org/2017/02/24/the-demonic-possession-of-hashie-mash.html is a good read.
2. Looking forward to your contribution! The idea of checking whether an actual ERB is being loaded is a fine one, but I would try not to be too clever, start simple, you can always add later.

[bo-tato]

Interesting read, thanks! I'm not too worried as I'm not using it in a program, it's just for interactive use in the shell. It's for a little script that can go in the middle of unix pipelines and open pry with the data. If the data is json it will load it into a Mash. I used to use jq but I can never remember the syntax. With ruby + mash it is almost as concise as jq which is nice for interactive data fiddling, more consistent and easier to remember, and more powerful as you have a full general purpose programming language

[michaelherold]

waxing nostalgic: Many moons ago, I set about ripping Mash.load out into a separate gem that would make the persistence step pluggable. I even created module_builder for that purpose.

Unfortunately, I never pushed that repository remotely and the hard drive on which that work lived died a scratchy death so I don't have that work anymore.

---

suggestion: Anyway, if you're only using it for accessing data and not writing it, perhaps hash_with_dot_access would work for your use case. It gives you the method access capabilities of Mash without a lot of the accompanying baggage.

It doesn't have a .load, but you could do something similar. Here's a bit of a hacky jq equivalent that you could call rq:

#!/usr/bin/env ruby

require "hash_with_dot_access"
require "json"

class Accessor < BasicObject
  def initialize(file)
    return unless ::File.exist?(file)

    @data = 
      ::File
      .read(file)
      .then { |content| ::JSON.parse(content) }
      .then { |json| ::HashWithDotAccess::Hash.new(json) }
  end

  def method_missing(method_name, ...)
    return super unless @data.key?(method_name)

    @data.__send__(method_name)
  end

  def respond_to_missing?(method_name, include_private = false)
    @data.key?(method_name) || super
  end
end

file_name, *commands = ARGV

puts Accessor.new(file_name).instance_eval(commands.join("")).inspect

For file blah.json:

{"my": {"deeply": ["nested", "data"]}}

You could do:

rq blah.json my.deeply

to get:

["nested", "data"]

It has a bunch of gotchas (e.g. Kernel has a #test method so any key test will cause it to explode), but this might give you some ideas!