Skip to content
New issue

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

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

Already on GitHub? # to your account

Sanitizing property names in queries and creation commands. #172

Merged
merged 1 commit into from
Feb 9, 2016

Conversation

obi1kenobi
Copy link
Contributor

In private discussion with @mogui we decided to resolve #169 via a pull request. The security issue was a SQL injection attack vector, exploitable in one location and potentially a few more, that allowed an attacker to change the WHERE clause in a query and cause it to return unexpected results. Here is an example query:

rat = g.animals.create(name='rat', specie='rodent')
mouse = g.animals.create(name='mouse', specie='rodent')

args = {'name': 'rat', 'name="rat" OR 1': 1}
res = g.animals.query(**args).all()
# Command: SELECT FROM animal WHERE name='rat' and name="rat" OR 1=1
#   Expected result: [
#        ('name': 'rat', 'specie': 'rodent')
#   ]
#   Actual result: [
#        ('name': 'rat', 'specie': 'rodent'),
#        ('name': 'mouse', 'specie': 'rodent')
#   ]

graph.create_vertex and graph.create_edge also do similar processing of unsanitized kwargs, but it's masked by the call to props_to_db so they are currently not exploitable, as far as I could determine.

This pull request resolves the problem by sanitizing kwarg arguments to the affected functions. Ideally, it would be merged immediately after #170, which fixes a bug where a newline character in a string field will unexpectedly terminate the query and cause an exception.

Thanks to the maintainers of this repository for promptly acting on my bug report!

mogui added a commit that referenced this pull request Feb 9, 2016
Sanitizing property names in queries and creation commands.
@mogui mogui merged commit b7535bd into mogui:develop Feb 9, 2016
@obi1kenobi obi1kenobi deleted the query_keyword_sanitization branch February 9, 2016 19:42
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants