Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
fenchu opened this issue May 23, 2023 · 3 comments
Closed

Comments

@fenchu
Copy link

fenchu commented May 23, 2023

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
Copy link

deadoverflow commented Jun 6, 2023

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
Copy link
Author

fenchu commented Jun 8, 2023

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.

@fenchu fenchu closed this as completed Jun 8, 2023
@deadoverflow
Copy link

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

Best regards, deadoverflow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants