Details
-
Epic
-
Status: Closed
-
Normal
-
Resolution: Fixed
-
None
-
None
-
None
-
Use YAML safely
-
customfield_10700 188744
-
Agent
-
Not Needed
-
Release notes not needed for the epic. See linked tickets for specific release notes.
-
Needs Assessment
Description
CVE-2017-2295 demonstrated that it's possible to use YAML on the network despite our attempts to not allow that. We fixed the immediate issue in PUP-7483, but the proper fix is to remove the YAML serialization code so that it cannot be accessed from a network context.
I propose we:
1. Add a load_data or parse_data method to Puppet::Util::Yaml so that it can safely load yaml from a string (or IO?) by stripping all tags prior to parsing it.
2. Implement a set of serializers for json, pson, yaml, msgpack, console, dot.
class Puppet::Serializer::Json |
def read_from(text) |
JSON.parse(text) |
end |
...
|
end
|
|
class Puppet::Serializer::Yaml |
include Puppet::Util::Yaml
|
|
def read_from(text) |
load_data(text)
|
end |
...
|
end |
3. Modify face_base to use these serializers instead of the network-based ones, e.g. when rendering console, yaml, json output, e.g. puppet facts find --render_as yaml
4. Modify network-based formats to delegate to the serializers something like:
Puppet::Network::FormatHandler.create_serialized_formats(:json, :mime => 'application/json', :weight => 15, :required_methods => [:render_method, :intern_method], :intern_method => :from_data_hash) do |
def intern(klass, text) |
serializer = Puppet::Serializer::Json.new |
data = serializer.read_from(text)
|
klass.from_data_hash(data)
|
end |
5. Remove unsupported network formats (console, yaml), probably dot too? Only json, pson, and msgpack should be reachable from network context.
6. Ensure all YAML calls, e.g. YAML.load_file go through Puppet::Util::Yaml
7. Some domain objects rely on YAML.load to construct nested objects which are contained in the hash passed to from_data_hash, e.g. https://github.com/puppetlabs/puppet/blob/4.10.0/lib/puppet/resource/status.rb#L197-L200. Modify from_data_hash/initialize_from_hash to consistently accept a hash, and reconstruct all child objects, so that from_data_hash and to_data_hash are symmetric.
/cc patrick