Skip to content

Commit 83c3917

Browse files
committed
[3.13] pythongh-139283: correctly handle size limit in cursor.fetchmany() (pythonGH-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. (cherry picked from commit bc172ee) Co-authored-by: Bénédikt Tran <[email protected]>
1 parent 6260b6a commit 83c3917

File tree

6 files changed

+150
-19
lines changed

6 files changed

+150
-19
lines changed

Doc/library/sqlite3.rst

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

1633+
.. versionchanged:: next
1634+
Negative *size* values are rejected by raising :exc:`ValueError`.
1635+
16331636
.. method:: fetchall()
16341637

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

1663+
.. versionchanged:: next
1664+
Negative values are rejected by raising :exc:`ValueError`.
1665+
16601666
.. attribute:: connection
16611667

16621668
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
@@ -1091,7 +1092,7 @@ def test_array_size(self):
10911092
# now set to 2
10921093
self.cu.arraysize = 2
10931094

1094-
# now make the query return 3 rows
1095+
# now make the query return 2 rows from a table of 3 rows
10951096
self.cu.execute("delete from test")
10961097
self.cu.execute("insert into test(name) values ('A')")
10971098
self.cu.execute("insert into test(name) values ('B')")
@@ -1101,13 +1102,50 @@ def test_array_size(self):
11011102

11021103
self.assertEqual(len(res), 2)
11031104

1105+
def test_invalid_array_size(self):
1106+
UINT32_MAX = (1 << 32) - 1
1107+
setter = functools.partial(setattr, self.cu, 'arraysize')
1108+
1109+
self.assertRaises(TypeError, setter, 1.0)
1110+
self.assertRaises(ValueError, setter, -3)
1111+
self.assertRaises(OverflowError, setter, UINT32_MAX + 1)
1112+
11041113
def test_fetchmany(self):
1114+
# no active SQL statement
1115+
res = self.cu.fetchmany()
1116+
self.assertEqual(res, [])
1117+
res = self.cu.fetchmany(1000)
1118+
self.assertEqual(res, [])
1119+
1120+
# test default parameter
1121+
self.cu.execute("select name from test")
1122+
res = self.cu.fetchmany()
1123+
self.assertEqual(len(res), 1)
1124+
1125+
# test when the number of requested rows exceeds the actual count
11051126
self.cu.execute("select name from test")
11061127
res = self.cu.fetchmany(100)
11071128
self.assertEqual(len(res), 1)
11081129
res = self.cu.fetchmany(100)
11091130
self.assertEqual(res, [])
11101131

1132+
# test when size = 0
1133+
self.cu.execute("select name from test")
1134+
res = self.cu.fetchmany(0)
1135+
self.assertEqual(res, [])
1136+
res = self.cu.fetchmany(100)
1137+
self.assertEqual(len(res), 1)
1138+
res = self.cu.fetchmany(100)
1139+
self.assertEqual(res, [])
1140+
1141+
def test_invalid_fetchmany(self):
1142+
UINT32_MAX = (1 << 32) - 1
1143+
fetchmany = self.cu.fetchmany
1144+
1145+
self.assertRaises(TypeError, fetchmany, 1.0)
1146+
self.assertRaises(ValueError, fetchmany, -3)
1147+
self.assertRaises(OverflowError, fetchmany, UINT32_MAX + 1)
1148+
11111149
def test_fetchmany_kw_arg(self):
11121150
"""Checks if fetchmany works with keyword arguments"""
11131151
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: 53 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1157,35 +1157,37 @@ pysqlite_cursor_fetchone_impl(pysqlite_Cursor *self)
11571157
/*[clinic input]
11581158
_sqlite3.Cursor.fetchmany as pysqlite_cursor_fetchmany
11591159
1160-
size as maxrows: int(c_default='self->arraysize') = 1
1160+
size as maxrows: size_t(c_default='((pysqlite_Cursor *)self)->arraysize') = 1
11611161
The default value is set by the Cursor.arraysize attribute.
11621162
11631163
Fetches several rows from the resultset.
11641164
[clinic start generated code]*/
11651165

11661166
static PyObject *
1167-
pysqlite_cursor_fetchmany_impl(pysqlite_Cursor *self, int maxrows)
1168-
/*[clinic end generated code: output=a8ef31fea64d0906 input=c26e6ca3f34debd0]*/
1167+
pysqlite_cursor_fetchmany_impl(pysqlite_Cursor *self, size_t maxrows)
1168+
/*[clinic end generated code: output=8bff46d4f24c9e84 input=5485f1f2ae46c2ff]*/
11691169
{
1170+
if (maxrows > (size_t)UINT32_MAX) {
1171+
PyErr_SetString(PyExc_OverflowError,
1172+
"Python int too large for C uint32_t");
1173+
return NULL;
1174+
}
1175+
11701176
PyObject* row;
11711177
PyObject* list;
1172-
int counter = 0;
11731178

11741179
list = PyList_New(0);
11751180
if (!list) {
11761181
return NULL;
11771182
}
11781183

1179-
while ((row = pysqlite_cursor_iternext(self))) {
1180-
if (PyList_Append(list, row) < 0) {
1181-
Py_DECREF(row);
1182-
break;
1183-
}
1184+
while (maxrows > 0 && (row = pysqlite_cursor_iternext((PyObject *)self))) {
1185+
int rc = PyList_Append(list, row);
11841186
Py_DECREF(row);
1185-
1186-
if (++counter == maxrows) {
1187+
if (rc < 0) {
11871188
break;
11881189
}
1190+
maxrows--;
11891191
}
11901192

11911193
if (PyErr_Occurred()) {
@@ -1299,6 +1301,40 @@ pysqlite_cursor_close_impl(pysqlite_Cursor *self)
12991301
Py_RETURN_NONE;
13001302
}
13011303

1304+
/*[clinic input]
1305+
@getter
1306+
_sqlite3.Cursor.arraysize
1307+
[clinic start generated code]*/
1308+
1309+
static PyObject *
1310+
_sqlite3_Cursor_arraysize_get_impl(pysqlite_Cursor *self)
1311+
/*[clinic end generated code: output=e0919d97175e6c50 input=3278f8d3ecbd90e3]*/
1312+
{
1313+
return PyLong_FromSize_t(self->arraysize);
1314+
}
1315+
1316+
/*[clinic input]
1317+
@setter
1318+
_sqlite3.Cursor.arraysize
1319+
[clinic start generated code]*/
1320+
1321+
static int
1322+
_sqlite3_Cursor_arraysize_set_impl(pysqlite_Cursor *self, PyObject *value)
1323+
/*[clinic end generated code: output=af59a6b09f8cce6e input=ace48cb114e26060]*/
1324+
{
1325+
size_t converted;
1326+
if (!_PyLong_Size_t_Converter(value, &converted)) {
1327+
return -1;
1328+
}
1329+
if (converted > (size_t)UINT32_MAX) {
1330+
PyErr_SetString(PyExc_OverflowError,
1331+
"Python int too large to convert to C uint32_t");
1332+
return -1;
1333+
}
1334+
self->arraysize = (uint32_t)converted;
1335+
return 0;
1336+
}
1337+
13021338
static PyMethodDef cursor_methods[] = {
13031339
PYSQLITE_CURSOR_CLOSE_METHODDEF
13041340
PYSQLITE_CURSOR_EXECUTEMANY_METHODDEF
@@ -1316,14 +1352,18 @@ static struct PyMemberDef cursor_members[] =
13161352
{
13171353
{"connection", _Py_T_OBJECT, offsetof(pysqlite_Cursor, connection), Py_READONLY},
13181354
{"description", _Py_T_OBJECT, offsetof(pysqlite_Cursor, description), Py_READONLY},
1319-
{"arraysize", Py_T_INT, offsetof(pysqlite_Cursor, arraysize), 0},
13201355
{"lastrowid", _Py_T_OBJECT, offsetof(pysqlite_Cursor, lastrowid), Py_READONLY},
13211356
{"rowcount", Py_T_LONG, offsetof(pysqlite_Cursor, rowcount), Py_READONLY},
13221357
{"row_factory", _Py_T_OBJECT, offsetof(pysqlite_Cursor, row_factory), 0},
13231358
{"__weaklistoffset__", Py_T_PYSSIZET, offsetof(pysqlite_Cursor, in_weakreflist), Py_READONLY},
13241359
{NULL}
13251360
};
13261361

1362+
static struct PyGetSetDef cursor_getsets[] = {
1363+
_SQLITE3_CURSOR_ARRAYSIZE_GETSETDEF
1364+
{NULL},
1365+
};
1366+
13271367
static const char cursor_doc[] =
13281368
PyDoc_STR("SQLite database cursor class.");
13291369

@@ -1334,6 +1374,7 @@ static PyType_Slot cursor_slots[] = {
13341374
{Py_tp_iternext, pysqlite_cursor_iternext},
13351375
{Py_tp_methods, cursor_methods},
13361376
{Py_tp_members, cursor_members},
1377+
{Py_tp_getset, cursor_getsets},
13371378
{Py_tp_init, pysqlite_cursor_init},
13381379
{Py_tp_traverse, cursor_traverse},
13391380
{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)