Is parsing a json naively into a Python class or struct secure?
Asked Answered
R

2

7

Some background first: I have a few rather simple data structures which are persisted as json files on disk. These json files are shared between applications of different languages and different environments (like web frontend and data manipulation tools).

For each of the files I want to create a Python "POPO" (Plain Old Python Object), and a corresponding data mapper class for each item should implement some simple CRUD like behavior (e.g. save will serialize the class and store as json file on disk).

I think a simple mapper (which only knows about basic types) will work. However, I'm concerned about security. Some of the json files will be generated by a web frontend, so a possible security risk if a user feeds me some bad json.

Finally, here is the simple mapping code (found at How to convert JSON data into a Python object):

class User(object):
def __init__(self, name, username):
    self.name = name
    self.username = username

import json
j = json.loads(your_json)
u = User(**j)

What possible security issues do you see?

NB: I'm new to Python.

Edit: Thanks all for your comments. I've found out that I have one json where I have 2 arrays, each having a map. Unfortunately this starts to look like it gets cumbersome when I get more of these.

I'm extending the question to mapping a json input to a recordtype. The original code is from here: https://mcmap.net/q/20837/-how-to-convert-json-data-into-a-python-object. Since I need mutable objects, I'd change it to use a namedlist instead of a namedtuple:

import json
from namedlist import namedlist

data = '{"name": "John Smith", "hometown": {"name": "New York", "id": 123}}'

# Parse JSON into an object with attributes corresponding to dict keys.
x = json.loads(data, object_hook=lambda d: namedlist('X', d.keys())(*d.values()))
print x.name, x.hometown.name, x.hometown.id

Is it still safe?

Reeves answered 5/7, 2015 at 22:53 Comment(3)
The only problem I can think of is that something will break because invalid data was loaded.Atrice
json can only parse limited types, int, bool, etc.. nothing will be executed so I don't see any real security riskMacron
I would still do some sanity checks on the input from the web service, especially if these objects are going to hit a database. Maybe something like limiting name to 100 printable characters and not allowing some punctuation (like semicolons). See this post #421546Bluhm
B
3

There's not much wrong that can happen in the first case. You're limiting what arguments can be provided and it's easy to add validation/conversion right after loading from JSON.

The second example is a bit worse. Packing things into records like this will not help you in any way. You don't inherit any methods, because each type you define is new. You can't compare values easily, because dicts are not ordered. You don't know if you have all arguments handled, or if there is some extra data, which can lead to hidden problems later.

So in summary: with User(**data), you're pretty safe. With namedlist there's space for ambiguity and you don't really gain anything. (compared to bare, parsed json)

Bale answered 6/7, 2015 at 0:37 Comment(0)
P
1

If you blindly accept users json input without sanity check, you are at risk of become json injection victim.

See detail explanation of json injection attack here: https://www.acunetix.com/blog/web-security-zone/what-are-json-injections/

Besides security vulnerability, parse JSON to Python object this way is not type safe.

With your example of User class, I would assume you expect both fields name and username to be string type. What if the json input is like this:

{
  "name": "my name",
  "username": 1
}
j = json.loads(your_json)
u = User(**j)

type(u.username) # int

You have gotten an object with unexpected type.

One solution to make sure type safe is to use json schema to validate input json. more about json schema: https://json-schema.org/

Pru answered 26/3, 2022 at 13:57 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.