JSON Encoder/Decoder Dangerous

Issue #248 resolved
Samuel Cochran
created an issue

0.19.0 has introduced a text encoder and decoder for JSON in the basic type mappes, woo!

Unfortunately it is using JSON.load and JSON.dump which are a little dangerous:

BEWARE: This method is meant to serialise data from trusted user input, like from your own database server or clients under your control, it could be dangerous to allow untrusted users to pass JSON sources into it. The default options for the parser can be changed via the ::load_default_options method.

This is because it includes the create_additions: true option by default which means if JSON additions are loaded then various Ruby classes can be instantiated during loading. While some folks might knowingly use this functionality, others might overlook it and leave their databases vulnerable to attack by malicious user input.

Here's an example:

require "pg"
require "json"
require "json/add/core"

conn = PG.connect(dbname: "test")

conn.type_map_for_queries = PG::BasicTypeMapForQueries.new conn
conn.type_map_for_results = PG::BasicTypeMapForResults.new conn

conn.exec("CREATE TEMP TABLE test (data JSON)")

conn.exec_params("INSERT INTO test VALUES ($1)", ['{"foo":{"json_class":"Symbol","s":"bar"}}'])

conn.exec("SELECT data FROM test").first["data"]
# => {"foo" => :bar}

These core additions might be mostly benign. Before symbol garbage collection in ruby 2.2 this might be an issue. But potentially more malicious are frameworks (cough cough) which might supply more complex JSON additions.

The fix is to switch to JSON.parse and JSON.stringify, optionally with the quirks_mode: true option to make it store bare values (null, true, 1, "foo") as well as valid JSON objects ({"foo":"bar"}). Or we could just supply explicit options to the load call instead of relying on the defaults. Perhaps this should even be configurable.

Comments (2)

  1. Lars Kanis

    Thanks for the great description of the issue! I've changed the code according to your suggestion.

    Please also note, that you can define your own type map or change the behavior of BasicTypeMapForResults like this:

    class MyJsonDeco < PG::SimpleDecoder
      def decode(string, tuple=nil, field=nil)
        p string
      end
    end
    PG::BasicTypeRegistry.register_type 0, 'json', nil, MyJsonDeco
    
  2. Log in to comment