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://stackoverflow.com/a/15882054/1708349. 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?

Community
  • 1
  • 1
benjist
  • 2,268
  • 2
  • 21
  • 48
  • The only problem I can think of is that something will break because invalid data was loaded. – poke Jul 05 '15 at 23:04
  • 3
    json can only parse limited types, int, bool, etc.. nothing will be executed so I don't see any real security risk – Padraic Cunningham Jul 05 '15 at 23:04
  • 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 http://stackoverflow.com/questions/421046/what-are-all-of-the-allowable-characters-for-peoples-names – Peter Gibson Jul 05 '15 at 23:22

1 Answers1

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)

viraptor
  • 30,857
  • 7
  • 96
  • 176