Skip to content

Commit 4f52cd7

Browse files
committed
Defensive mode: Initialize factories in tp_new,
re-initialize in tp_init (because of tp_clear calls) and also guard each use of row_factory and text_factory with an explicit NULL check. Keep the previous delattr guards, because who would be calling __delattr__ on these anyway. I'm imagining someone will suggest removing some of these changes, but something something seek forgiveness something.
1 parent f560467 commit 4f52cd7

4 files changed

Lines changed: 76 additions & 13 deletions

File tree

Lib/test/test_sqlite3/test_factory.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,35 @@ def test_delete_cursor_row_factory(self):
164164
# Executing a query here should succeed.
165165
self.assertEqual(tuple(cur.execute("select 1").fetchone()), (1,))
166166

167+
def test_uninitialized_connection_factories(self):
168+
# gh-152817: skipping __init__() should still result in initialized factories (None not Null)
169+
con = sqlite.Connection.__new__(sqlite.Connection)
170+
self.assertIsNone(con.row_factory)
171+
self.assertIs(con.text_factory, str)
172+
173+
def test_uninitialized_cursor_row_factory(self):
174+
# gh-152817: skipping __init__() should still result in initialized factories (None not Null)
175+
# __init__ must not crash.
176+
cur = sqlite.Cursor.__new__(sqlite.Cursor)
177+
self.assertIsNone(cur.row_factory)
178+
179+
def test_subclass_skipping_super_init(self):
180+
# gh-152817: forgetting to call super().__init__() shouldn't leave a NULL {row,text}_factory
181+
class Connection(sqlite.Connection):
182+
def __init__(self, *args, **kwargs):
183+
pass
184+
185+
class Cursor(sqlite.Cursor):
186+
def __init__(self, *args, **kwargs):
187+
pass
188+
189+
con = Connection(":memory:")
190+
self.assertIsNone(con.row_factory)
191+
self.assertIs(con.text_factory, str)
192+
193+
cur = Cursor(self.con)
194+
self.assertIsNone(cur.row_factory)
195+
167196
def test_sqlite_row_index_unicode(self):
168197
row = self.con.execute("select 1 as \xff").fetchone()
169198
self.assertEqual(row["\xff"], 1)
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
:mod:`sqlite3`: Disallow removing ``row_factory`` and ``text_factory`` attributes
2-
of a connection or cursor to prevent a crash on a query.
1+
:mod:`sqlite3`: Prevent crashes caused by row_factory or text_factory being uninitialized, either
2+
by skipping __init__ or by deleting the attributes.

Modules/_sqlite/connection.c

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -300,8 +300,9 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, PyObject *database,
300300
self->thread_ident = PyThread_get_thread_ident();
301301
self->statement_cache = statement_cache;
302302
self->blobs = blobs;
303-
self->row_factory = Py_NewRef(Py_None);
304-
self->text_factory = Py_NewRef(&PyUnicode_Type);
303+
// re-initialize the factory members here, as tp_clear() is called above in some cases
304+
Py_XSETREF(self->row_factory, Py_NewRef(Py_None));
305+
Py_XSETREF(self->text_factory, Py_NewRef((PyObject *)&PyUnicode_Type));
305306
self->trace_ctx = NULL;
306307
self->progress_ctx = NULL;
307308
self->authorizer_ctx = NULL;
@@ -339,6 +340,19 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, PyObject *database,
339340
return -1;
340341
}
341342

343+
static PyObject *
344+
pysqlite_connection_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
345+
{
346+
pysqlite_Connection *self = (pysqlite_Connection *)type->tp_alloc(type, 0);
347+
if (self == NULL) {
348+
return NULL;
349+
}
350+
// row_factory and text_factory should never be uninitialized, even if tp_init is bypassed.
351+
self->row_factory = Py_NewRef(Py_None);
352+
self->text_factory = Py_NewRef((PyObject *)&PyUnicode_Type);
353+
return (PyObject *)self;
354+
}
355+
342356
/*[clinic input]
343357
# Create a new destination 'connect' for the docstring and methoddef only.
344358
# This makes it possible to keep the signatures for Connection.__init__ and
@@ -549,7 +563,7 @@ pysqlite_connection_cursor_impl(pysqlite_Connection *self, PyObject *factory)
549563
return NULL;
550564
}
551565

552-
if (cursor && self->row_factory != Py_None) {
566+
if (cursor && self->row_factory != NULL && !Py_IsNone(self->row_factory)) {
553567
Py_INCREF(self->row_factory);
554568
Py_XSETREF(((pysqlite_Cursor *)cursor)->row_factory, self->row_factory);
555569
}
@@ -561,7 +575,8 @@ static PyObject *
561575
connection_get_row_factory(PyObject *op, void *closure)
562576
{
563577
pysqlite_Connection *self = (pysqlite_Connection *)op;
564-
return Py_NewRef(self->row_factory);
578+
PyObject *row_factory = self->row_factory;
579+
return Py_NewRef(row_factory != NULL ? row_factory : Py_None);
565580
}
566581

567582
static int
@@ -581,7 +596,9 @@ static PyObject *
581596
connection_get_text_factory(PyObject *op, void *closure)
582597
{
583598
pysqlite_Connection *self = (pysqlite_Connection *)op;
584-
return Py_NewRef(self->text_factory);
599+
PyObject *text_factory = self->text_factory;
600+
return Py_NewRef(text_factory != NULL ? text_factory
601+
: (PyObject *)&PyUnicode_Type);
585602
}
586603

587604
static int
@@ -2722,6 +2739,7 @@ static PyType_Slot connection_slots[] = {
27222739
{Py_tp_methods, connection_methods},
27232740
{Py_tp_members, connection_members},
27242741
{Py_tp_getset, connection_getset},
2742+
{Py_tp_new, pysqlite_connection_new},
27252743
{Py_tp_init, pysqlite_connection_init},
27262744
{Py_tp_call, pysqlite_connection_call},
27272745
{Py_tp_traverse, connection_traverse},

Modules/_sqlite/cursor.c

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,18 @@ pysqlite_cursor_init_impl(pysqlite_Cursor *self,
143143
return 0;
144144
}
145145

146+
static PyObject *
147+
pysqlite_cursor_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
148+
{
149+
pysqlite_Cursor *self = (pysqlite_Cursor *)type->tp_alloc(type, 0);
150+
if (self == NULL) {
151+
return NULL;
152+
}
153+
// row_factory should never be uninitialized, even if tp_init is bypassed.
154+
self->row_factory = Py_NewRef(Py_None);
155+
return (PyObject *)self;
156+
}
157+
146158
static inline int
147159
stmt_reset(pysqlite_Statement *self)
148160
{
@@ -413,7 +425,9 @@ _pysqlite_fetch_one_row(pysqlite_Cursor* self)
413425
}
414426

415427
nbytes = sqlite3_column_bytes(self->statement->st, i);
416-
if (self->connection->text_factory == (PyObject*)&PyUnicode_Type) {
428+
PyObject *text_factory = self->connection->text_factory;
429+
if (text_factory == NULL ||
430+
text_factory == (PyObject*)&PyUnicode_Type) {
417431
converted = PyUnicode_FromStringAndSize(text, nbytes);
418432
if (!converted && PyErr_ExceptionMatches(PyExc_UnicodeDecodeError)) {
419433
PyErr_Clear();
@@ -434,12 +448,12 @@ _pysqlite_fetch_one_row(pysqlite_Cursor* self)
434448
Py_DECREF(error_msg);
435449
}
436450
}
437-
} else if (self->connection->text_factory == (PyObject*)&PyBytes_Type) {
451+
} else if (text_factory == (PyObject*)&PyBytes_Type) {
438452
converted = PyBytes_FromStringAndSize(text, nbytes);
439-
} else if (self->connection->text_factory == (PyObject*)&PyByteArray_Type) {
453+
} else if (text_factory == (PyObject*)&PyByteArray_Type) {
440454
converted = PyByteArray_FromStringAndSize(text, nbytes);
441455
} else {
442-
converted = PyObject_CallFunction(self->connection->text_factory, "y#", text, nbytes);
456+
converted = PyObject_CallFunction(text_factory, "y#", text, nbytes);
443457
}
444458
} else {
445459
/* coltype == SQLITE_BLOB */
@@ -1176,7 +1190,7 @@ pysqlite_cursor_iternext(PyObject *op)
11761190
}
11771191
return NULL;
11781192
}
1179-
if (!Py_IsNone(self->row_factory)) {
1193+
if (self->row_factory != NULL && !Py_IsNone(self->row_factory)) {
11801194
PyObject *factory = self->row_factory;
11811195
PyObject *args[] = { op, row, };
11821196
PyObject *new_row = PyObject_Vectorcall(factory, args, 2, NULL);
@@ -1408,7 +1422,8 @@ static PyObject *
14081422
cursor_get_row_factory(PyObject *op, void *closure)
14091423
{
14101424
pysqlite_Cursor *self = (pysqlite_Cursor *)op;
1411-
return Py_NewRef(self->row_factory);
1425+
PyObject *row_factory = self->row_factory;
1426+
return Py_NewRef(row_factory != NULL ? row_factory : Py_None);
14121427
}
14131428

14141429
static int
@@ -1441,6 +1456,7 @@ static PyType_Slot cursor_slots[] = {
14411456
{Py_tp_methods, cursor_methods},
14421457
{Py_tp_members, cursor_members},
14431458
{Py_tp_getset, cursor_getsets},
1459+
{Py_tp_new, pysqlite_cursor_new},
14441460
{Py_tp_init, pysqlite_cursor_init},
14451461
{Py_tp_traverse, cursor_traverse},
14461462
{Py_tp_clear, cursor_clear},

0 commit comments

Comments
 (0)