Skip to content

Commit bc172ee

Browse files
authored
gh-139283: correctly handle size limit in cursor.fetchmany() (#139296)
Passing a negative or zero size to `cursor.fetchmany()` made it fetch all rows instead of none. While this could be considered a security vulnerability, it was decided to treat this issue as a regular bug as passing a non-sanitized *size* value in the first place is not recommended.
1 parent bd1ada6 commit bc172ee

File tree

6 files changed

+134
-19
lines changed

6 files changed

+134
-19
lines changed

Doc/library/sqlite3.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1611,6 +1611,9 @@ Cursor objects
16111611
If the *size* parameter is used, then it is best for it to retain the same
16121612
value from one :meth:`fetchmany` call to the next.
16131613

1614+
.. versionchanged:: next
1615+
Negative *size* values are rejected by raising :exc:`ValueError`.
1616+
16141617
.. method:: fetchall()
16151618

16161619
Return all (remaining) rows of a query result as a :class:`list`.
@@ -1638,6 +1641,9 @@ Cursor objects
16381641
Read/write attribute that controls the number of rows returned by :meth:`fetchmany`.
16391642
The default value is 1 which means a single row would be fetched per call.
16401643

1644+
.. versionchanged:: next
1645+
Negative values are rejected by raising :exc:`ValueError`.
1646+
16411647
.. attribute:: connection
16421648

16431649
Read-only attribute that provides the SQLite database :class:`Connection`

Lib/test/test_sqlite3/test_dbapi.py

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
# 3. This notice may not be removed or altered from any source distribution.
2222

2323
import contextlib
24+
import functools
2425
import os
2526
import sqlite3 as sqlite
2627
import subprocess
@@ -1060,7 +1061,7 @@ def test_array_size(self):
10601061
# now set to 2
10611062
self.cu.arraysize = 2
10621063

1063-
# now make the query return 3 rows
1064+
# now make the query return 2 rows from a table of 3 rows
10641065
self.cu.execute("delete from test")
10651066
self.cu.execute("insert into test(name) values ('A')")
10661067
self.cu.execute("insert into test(name) values ('B')")
@@ -1070,13 +1071,50 @@ def test_array_size(self):
10701071

10711072
self.assertEqual(len(res), 2)
10721073

1074+
def test_invalid_array_size(self):
1075+
UINT32_MAX = (1 << 32) - 1
1076+
setter = functools.partial(setattr, self.cu, 'arraysize')
1077+
1078+
self.assertRaises(TypeError, setter, 1.0)
1079+
self.assertRaises(ValueError, setter, -3)
1080+
self.assertRaises(OverflowError, setter, UINT32_MAX + 1)
1081+
10731082
def test_fetchmany(self):
1083+
# no active SQL statement
1084+
res = self.cu.fetchmany()
1085+
self.assertEqual(res, [])
1086+
res = self.cu.fetchmany(1000)
1087+
self.assertEqual(res, [])
1088+
1089+
# test default parameter
1090+
self.cu.execute("select name from test")
1091+
res = self.cu.fetchmany()
1092+
self.assertEqual(len(res), 1)
1093+
1094+
# test when the number of requested rows exceeds the actual count
10741095
self.cu.execute("select name from test")
10751096
res = self.cu.fetchmany(100)
10761097
self.assertEqual(len(res), 1)
10771098
res = self.cu.fetchmany(100)
10781099
self.assertEqual(res, [])
10791100

1101+
# test when size = 0
1102+
self.cu.execute("select name from test")
1103+
res = self.cu.fetchmany(0)
1104+
self.assertEqual(res, [])
1105+
res = self.cu.fetchmany(100)
1106+
self.assertEqual(len(res), 1)
1107+
res = self.cu.fetchmany(100)
1108+
self.assertEqual(res, [])
1109+
1110+
def test_invalid_fetchmany(self):
1111+
UINT32_MAX = (1 << 32) - 1
1112+
fetchmany = self.cu.fetchmany
1113+
1114+
self.assertRaises(TypeError, fetchmany, 1.0)
1115+
self.assertRaises(ValueError, fetchmany, -3)
1116+
self.assertRaises(OverflowError, fetchmany, UINT32_MAX + 1)
1117+
10801118
def test_fetchmany_kw_arg(self):
10811119
"""Checks if fetchmany works with keyword arguments"""
10821120
self.cu.execute("select name from test")
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
:mod:`sqlite3`: correctly handle maximum number of rows to fetch in
2+
:meth:`Cursor.fetchmany <sqlite3.Cursor.fetchmany>` and reject negative
3+
values for :attr:`Cursor.arraysize <sqlite3.Cursor.arraysize>`. Patch by
4+
Bénédikt Tran.

Modules/_sqlite/clinic/cursor.c.h

Lines changed: 47 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Modules/_sqlite/cursor.c

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1159,35 +1159,31 @@ pysqlite_cursor_fetchone_impl(pysqlite_Cursor *self)
11591159
/*[clinic input]
11601160
_sqlite3.Cursor.fetchmany as pysqlite_cursor_fetchmany
11611161
1162-
size as maxrows: int(c_default='((pysqlite_Cursor *)self)->arraysize') = 1
1162+
size as maxrows: uint32(c_default='((pysqlite_Cursor *)self)->arraysize') = 1
11631163
The default value is set by the Cursor.arraysize attribute.
11641164
11651165
Fetches several rows from the resultset.
11661166
[clinic start generated code]*/
11671167

11681168
static PyObject *
1169-
pysqlite_cursor_fetchmany_impl(pysqlite_Cursor *self, int maxrows)
1170-
/*[clinic end generated code: output=a8ef31fea64d0906 input=035dbe44a1005bf2]*/
1169+
pysqlite_cursor_fetchmany_impl(pysqlite_Cursor *self, uint32_t maxrows)
1170+
/*[clinic end generated code: output=3325f2b477c71baf input=a509c412aa70b27e]*/
11711171
{
11721172
PyObject* row;
11731173
PyObject* list;
1174-
int counter = 0;
11751174

11761175
list = PyList_New(0);
11771176
if (!list) {
11781177
return NULL;
11791178
}
11801179

1181-
while ((row = pysqlite_cursor_iternext((PyObject *)self))) {
1182-
if (PyList_Append(list, row) < 0) {
1183-
Py_DECREF(row);
1184-
break;
1185-
}
1180+
while (maxrows > 0 && (row = pysqlite_cursor_iternext((PyObject *)self))) {
1181+
int rc = PyList_Append(list, row);
11861182
Py_DECREF(row);
1187-
1188-
if (++counter == maxrows) {
1183+
if (rc < 0) {
11891184
break;
11901185
}
1186+
maxrows--;
11911187
}
11921188

11931189
if (PyErr_Occurred()) {
@@ -1301,6 +1297,30 @@ pysqlite_cursor_close_impl(pysqlite_Cursor *self)
13011297
Py_RETURN_NONE;
13021298
}
13031299

1300+
/*[clinic input]
1301+
@getter
1302+
_sqlite3.Cursor.arraysize
1303+
[clinic start generated code]*/
1304+
1305+
static PyObject *
1306+
_sqlite3_Cursor_arraysize_get_impl(pysqlite_Cursor *self)
1307+
/*[clinic end generated code: output=e0919d97175e6c50 input=3278f8d3ecbd90e3]*/
1308+
{
1309+
return PyLong_FromUInt32(self->arraysize);
1310+
}
1311+
1312+
/*[clinic input]
1313+
@setter
1314+
_sqlite3.Cursor.arraysize
1315+
[clinic start generated code]*/
1316+
1317+
static int
1318+
_sqlite3_Cursor_arraysize_set_impl(pysqlite_Cursor *self, PyObject *value)
1319+
/*[clinic end generated code: output=af59a6b09f8cce6e input=ace48cb114e26060]*/
1320+
{
1321+
return PyLong_AsUInt32(value, &self->arraysize);
1322+
}
1323+
13041324
static PyMethodDef cursor_methods[] = {
13051325
PYSQLITE_CURSOR_CLOSE_METHODDEF
13061326
PYSQLITE_CURSOR_EXECUTEMANY_METHODDEF
@@ -1318,14 +1338,18 @@ static struct PyMemberDef cursor_members[] =
13181338
{
13191339
{"connection", _Py_T_OBJECT, offsetof(pysqlite_Cursor, connection), Py_READONLY},
13201340
{"description", _Py_T_OBJECT, offsetof(pysqlite_Cursor, description), Py_READONLY},
1321-
{"arraysize", Py_T_INT, offsetof(pysqlite_Cursor, arraysize), 0},
13221341
{"lastrowid", _Py_T_OBJECT, offsetof(pysqlite_Cursor, lastrowid), Py_READONLY},
13231342
{"rowcount", Py_T_LONG, offsetof(pysqlite_Cursor, rowcount), Py_READONLY},
13241343
{"row_factory", _Py_T_OBJECT, offsetof(pysqlite_Cursor, row_factory), 0},
13251344
{"__weaklistoffset__", Py_T_PYSSIZET, offsetof(pysqlite_Cursor, in_weakreflist), Py_READONLY},
13261345
{NULL}
13271346
};
13281347

1348+
static struct PyGetSetDef cursor_getsets[] = {
1349+
_SQLITE3_CURSOR_ARRAYSIZE_GETSETDEF
1350+
{NULL},
1351+
};
1352+
13291353
static const char cursor_doc[] =
13301354
PyDoc_STR("SQLite database cursor class.");
13311355

@@ -1336,6 +1360,7 @@ static PyType_Slot cursor_slots[] = {
13361360
{Py_tp_iternext, pysqlite_cursor_iternext},
13371361
{Py_tp_methods, cursor_methods},
13381362
{Py_tp_members, cursor_members},
1363+
{Py_tp_getset, cursor_getsets},
13391364
{Py_tp_init, pysqlite_cursor_init},
13401365
{Py_tp_traverse, cursor_traverse},
13411366
{Py_tp_clear, cursor_clear},

Modules/_sqlite/cursor.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ typedef struct
3535
pysqlite_Connection* connection;
3636
PyObject* description;
3737
PyObject* row_cast_map;
38-
int arraysize;
38+
uint32_t arraysize;
3939
PyObject* lastrowid;
4040
long rowcount;
4141
PyObject* row_factory;

0 commit comments

Comments
 (0)