msiemens / tinydb

TinyDB is a lightweight document oriented database optimized for your happiness :)
https://tinydb.readthedocs.org
MIT License
6.76k stars 534 forks source link

de-serialization Query() statement for search is there a way not use eval #528

Closed fenchu closed 1 year ago

fenchu commented 1 year ago

I just looked at tindydb for a simple backward lookup-table in our gitlab job-board.

Works fine, but Snykt (our security analysis software) bails at the eval I have used. Eval is terrible, but eval in an input parameter on a rest api it a no-go.

Any way to omit the eval?

def read(s:str) -> Optional[Dict]:
    """ query jobid and return struct if exists, else it return an empty list """
    global db
    if not db:
        db = TinyDB(db_path)
    return db.search(eval(f"Query().{s}"))

calling it read("jobid==1139998")) returning:

[{'jobid': 119998, 'refs': {'allure': '<allureurl>', 'filestore': '<filestoreurl>', 'jenkins': '<jenkinsurl>', 'reportportal': '<reportportalurl>'}}]

This is called from a rest-api so I have to send a string query not a statement.

deadoverflow commented 1 year ago

yeah based on the function you provided there is a code execution bug in the function read(). The eval function is a problem here because if you call read("value=='test' and print(7*7)") you would see 49 in the shell as well as some weird ass error but the code was executed!

from tinydb import TinyDB, Query

db = TinyDB('tests.json')
db.insert({
    'value': 'test'
})

def read(s:str):
    """ query jobid and return struct if exists, else it return an empty list """
    global db
    return db.search(eval(f"Query().{s}"))

print(read("value=='test' and print(7*7)"))

Here is the code that i have used and this is my console after i ran this:

image

Better way to write this read function is to add sanitization and filters so that it detects any malicious data. For example this is the example of safer read function:

from tinydb import TinyDB, Query

def read(s:str):
    global db
    obj = s.split('==')
    key = obj[0]
    value = obj[1]
    if '(' in value and ')' in value or '(' in key and ')' in key:
        return []
    else:
        return eval(f'db.search(Query().{key} == {value})')

This function is restricting any use of functions in the eval which is good enough i guess. Code still can be executed but there is nothing to worry about since no functions can be called. I hope this helps with you understand why the eval is bad and that your function had a deadly bug!

fenchu commented 1 year ago

Thanks I will use that, but I was hoping I could just write db.search(Query()."jobid==6830997") without the eval like json.loads()

Maybe a future feature request :-)

Thanks again, solution works as a dream, we keep 10K backlog links here now.

deadoverflow commented 1 year ago

no problem fenchu, i am glad i was able to solve your problem!

Best regards, deadoverflow