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.

Sign up for GitHub

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

Jump to bottom

Prepared statement parameterization failure on multiple-item inserts #30

Open
jaysonlarose opened this issue Jul 30, 2019 · 1 comment
Open

Prepared statement parameterization failure on multiple-item inserts #30

jaysonlarose opened this issue Jul 30, 2019 · 1 comment

Comments

@jaysonlarose
Copy link

Hello,

I just ran into this issue when trying to insert a large amount of rows into an rqlite database using multiple-item INSERTs.

Example code:

#!/usr/bin/env python3

import pyrqlite.dbapi2

if __name__ == '__main__':
	con = pyrqlite.dbapi2.connect(host="127.0.0.1", port=4001)
	cur = con.cursor()
	cur.execute("CREATE TABLE bug_demonstration (id INT UNSIGNED PRIMARY KEY, name TEXT)")
	con.commit()
	cur.execute("INSERT INTO bug_demonstration (name) VALUES (?), (?)", ['why am I being seen as a substitution token?', "little bobby tables sends his regards"])

When run against a stock rqlite server compiled from the current GitHub code and run in a completely vanilla fashion (rqlited bug_database), I get the following error:

{"error": "near \"little\": syntax error"}
Traceback (most recent call last):
  File "pyrqlite_bug.py", line 10, in <module>
    cur.execute("INSERT INTO bug_demonstration (name) VALUES (?), (?)", ['why am I being seen as a substitution token?', "little bobby tables sends his regards"])
  File "/usr/local/lib/python3.6/dist-packages/pyrqlite-HEAD-py3.6.egg/pyrqlite/cursors.py", line 178, in execute
sqlite3.Error: {"error": "near \"little\": syntax error"}

I used Wireshark to investigate the request coming down the line, which looked like this:

POST /db/execute?transaction HTTP/1.1
Host: 127.0.0.1:4001
Accept-Encoding: identity
Content-Length: 139
Content-Type: application/json

["INSERT INTO bug_demonstration (name) VALUES ('why am I being seen as a substitution token'little bobby tables sends his regards''), (?)"]

I figured it'd be a good idea to report this, as any parameterization or data-escaping errors have serious security implications.

Cheers,
--Jays
@zmedico
Copy link
Collaborator

zmedico commented Feb 28, 2021

@davidc0le the issue reported is from things like this in #25:

            for i in range(len(parameters)):
                operation = operation.replace('?',

zmedico added a commit to zmedico/pyrqlite that referenced this issue Feb 28, 2021
@zmedico
_substitute_params: fix incorrect parameterization for parameter sequ…
9c27189 
…ence

Fix incorrect parameterization for parameter sequence, by
restoring necessary code from before 5b5fb66.

This unit test case demonstrates the problem reported in
rqlite#30:

    def test_CheckRowcountExecute(self):
        self.cu.execute("delete from test")
        self.cu.execute("insert into test(name, income) values (?, ?)", ("?", "1"))
        self.cu.execute("select name from test where name=?", ("?",))
        self.assertEqual(self.cu.rowcount, 1,
            msg="test failed for rqlite#30")

src/test/test_dbapi.py:346:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <pyrqlite.cursors.Cursor object at 0x7f16e1abc400>, operation = "insert into test(name, income) values (''1'', ?)", parameters = ('?', '1')

    def execute(self, operation, parameters=None):
        if not isinstance(operation, basestring):
            raise ValueError(
                             "argument must be a string, not '{}'".format(type(operation).__name__))

        operation = self._substitute_params(operation, parameters)

        command = self._get_sql_command(operation)
        if command in ('SELECT', 'PRAGMA'):
            payload = self._request("GET",
                                    "/db/query?" + _urlencode({'q': operation}))
        else:
            payload = self._request("POST", "/db/execute?transaction",
                                    headers={'Content-Type': 'application/json'}, body=json.dumps([operation]))

        last_insert_id = None
        rows_affected = -1
        payload_rows = {}
        try:
            results = payload["results"]
        except KeyError:
            pass
        else:
            rows_affected = 0
            for item in results:
                if 'error' in item:
                    logging.getLogger(__name__).error(json.dumps(item))
>                   raise Error(json.dumps(item))
E                   sqlite3.Error: {"error": "near \"1\": syntax error"}

pyrqlite/cursors.py:178: Error

Note that a similar problem still exists for named
parameters, so rqlite#30
is not entirely fixed.

Reported-by: @jaysonlarose
See: rqlite#30
Fixes: 5b5fb66 ("Adding support for named parameters")
zmedico added a commit to zmedico/pyrqlite that referenced this issue Feb 28, 2021
@zmedico
_substitute_params: restore parameter sequence code from prior to 5b5…
d30fc76 
…fb66

Restore parameter sequence code from prior to commit
5b5fb66. This unit test case demonstrates the problem
reported in rqlite#30:

    def test_CheckRowcountExecute(self):
        self.cu.execute("delete from test")
        self.cu.execute("insert into test(name, income) values (?, ?)", ("?", "1"))
        self.cu.execute("select name from test where name=?", ("?",))
        self.assertEqual(self.cu.rowcount, 1,
            msg="test failed for rqlite#30")

src/test/test_dbapi.py:346:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <pyrqlite.cursors.Cursor object at 0x7f16e1abc400>, operation = "insert into test(name, income) values (''1'', ?)", parameters = ('?', '1')

    def execute(self, operation, parameters=None):
        if not isinstance(operation, basestring):
            raise ValueError(
                             "argument must be a string, not '{}'".format(type(operation).__name__))

        operation = self._substitute_params(operation, parameters)

        command = self._get_sql_command(operation)
        if command in ('SELECT', 'PRAGMA'):
            payload = self._request("GET",
                                    "/db/query?" + _urlencode({'q': operation}))
        else:
            payload = self._request("POST", "/db/execute?transaction",
                                    headers={'Content-Type': 'application/json'}, body=json.dumps([operation]))

        last_insert_id = None
        rows_affected = -1
        payload_rows = {}
        try:
            results = payload["results"]
        except KeyError:
            pass
        else:
            rows_affected = 0
            for item in results:
                if 'error' in item:
                    logging.getLogger(__name__).error(json.dumps(item))
>                   raise Error(json.dumps(item))
E                   sqlite3.Error: {"error": "near \"1\": syntax error"}

pyrqlite/cursors.py:178: Error

Note that a similar problem still exists for named
parameters, so rqlite#30
is not entirely fixed.

Reported-by: @jaysonlarose
See: rqlite#30
Fixes: 5b5fb66 ("Adding support for named parameters")
zmedico added a commit to zmedico/pyrqlite that referenced this issue Feb 28, 2021
@zmedico
_substitute_params: restore parameter sequence code from prior to 5b5…
b0bc6ab 
…fb66

Restore parameter sequence code from prior to commit
5b5fb66. This unit test case demonstrates the problem
reported in rqlite#30:

    def test_CheckRowcountExecute(self):
        self.cu.execute("delete from test")
        self.cu.execute("insert into test(name, income) values (?, ?)", ("?", "1"))
        self.cu.execute("select name from test where name=?", ("?",))
        self.assertEqual(self.cu.rowcount, 1,
            msg="test failed for rqlite#30")

src/test/test_dbapi.py:346:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <pyrqlite.cursors.Cursor object at 0x7f16e1abc400>, operation = "insert into test(name, income) values (''1'', ?)", parameters = ('?', '1')

    def execute(self, operation, parameters=None):
        if not isinstance(operation, basestring):
            raise ValueError(
                             "argument must be a string, not '{}'".format(type(operation).__name__))

        operation = self._substitute_params(operation, parameters)

        command = self._get_sql_command(operation)
        if command in ('SELECT', 'PRAGMA'):
            payload = self._request("GET",
                                    "/db/query?" + _urlencode({'q': operation}))
        else:
            payload = self._request("POST", "/db/execute?transaction",
                                    headers={'Content-Type': 'application/json'}, body=json.dumps([operation]))

        last_insert_id = None
        rows_affected = -1
        payload_rows = {}
        try:
            results = payload["results"]
        except KeyError:
            pass
        else:
            rows_affected = 0
            for item in results:
                if 'error' in item:
                    logging.getLogger(__name__).error(json.dumps(item))
>                   raise Error(json.dumps(item))
E                   sqlite3.Error: {"error": "near \"1\": syntax error"}

pyrqlite/cursors.py:178: Error

Note that a similar problem still exists for named
parameters, so rqlite#30
is not entirely fixed.

Reported-by: @jaysonlarose
See: rqlite#30
Fixes: 5b5fb66 ("Adding support for named parameters")
@zmedico zmedico mentioned this issue Feb 28, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@zmedico @jaysonlarose