Fix json_marshaller Universal Deserialisation Gadget of user-controlled data

[odaysec]

newrelic-ruby-agent/lib/new_relic/agent/new_relic_service/json_marshaller.rb

Lines 45 to 46 in a19f26d

	return_value(::JSON.load(data))
	rescue => e

To fix the issue, replace the unsafe JSON.load method with the safer JSON.parse method. The JSON.parse method does not allow the deserialization of arbitrary objects, making it a secure alternative for handling untrusted JSON data. Additionally, ensure that the input data is validated or sanitized before parsing.

The changes will be made in the load method of the JsonMarshaller class in lib/new_relic/agent/new_relic_service/json_marshaller.rb. Specifically:

1. Replace JSON.load(data) with JSON.parse(data).
2. Ensure that the data is a valid JSON string before parsing.

---

Deserializing untrusted data using any method that allows the construction of arbitrary objects is easily exploitable and, in many cases, allows an attacker to execute arbitrary code.

POC

The following calls the Marshal.load, JSON.load, YAML.load, Oj.load and Ox.parse_obj methods on data from an HTTP request. Since these methods are capable of deserializing to arbitrary objects, this is inherently unsafe.

require 'json'
require 'yaml'
require 'oj'

class UserController < ActionController::Base
  def marshal_example
    data = Base64.decode64 params[:data]
    object = Marshal.load data
    # ...
  end

  def json_example
    object = JSON.load params[:json]
    # ...
  end

  def yaml_example
    object = YAML.load params[:yaml]
    # ...
  end

  def oj_example
    object = Oj.load params[:json]
    # ...
  end

  def ox_example
    object = Ox.parse_obj params[:xml]
    # ...
  end
end

Using JSON.parse and YAML.safe_load instead, as in the following example, removes the vulnerability. Similarly, calling Oj.load with any mode other than :object is safe, as is calling Oj.safe_load. Note that there is no safe way to deserialize untrusted data using Marshal.

require 'json'

class UserController < ActionController::Base
  def safe_json_example
    object = JSON.parse params[:json]
    # ...
  end

  def safe_yaml_example
    object = YAML.safe_load params[:yaml]
    # ...
  end

  def safe_oj_example
    object = Oj.load params[:yaml], { mode: :strict }
    # or
    object = Oj.safe_load params[:yaml]
    # ...
  end
end

References

• deserialization of untrusted data
• guidance on deserializing objects safely
• security guidance on the Marshal library
• security guidance on JSON.load.
• security guidance on the YAML library
• Universal Deserialisation Gadget for Ruby 2.x-3.x
• CWE-502

Overview

Describe the changes present in the pull request

Submitter Checklist:

• Include a link to the related GitHub issue, if applicable
• Include a security review link, if applicable

Testing

The agent includes a suite of unit and functional tests which should be used to
verify your changes don't break existing functionality. These tests will run with
GitHub Actions when a pull request is made. More details on running the tests locally can be found
here for our unit tests,
and here for our functional tests.
For most contributions it is strongly recommended to add additional tests which
exercise your changes.

Reviewer Checklist

• Perform code review
• Add performance label
• Perform appropriate level of performance testing
• Confirm all checks passed
• Add version label prior to acceptance

[CLAassistant]

All committers have signed the CLA.

[kaylareopelle]

Hi @odaysec! Checking in on this PR. I see a few errors related to HTTPX, but I think those are unrelated. Would you mind merging dev into your branch?