Skip to content

Commit ba7d736

Browse files
orenmnserhiy-storchaka
orenmn
authored and
serhiy-storchaka
committed
bpo-31243: Fixed PyArg_ParseTuple failure checks. (#3171)
1 parent e9d978f commit ba7d736

File tree

4 files changed

+65
-21
lines changed

4 files changed

+65
-21
lines changed

‎Lib/test/test_io.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3253,6 +3253,26 @@ def _make_illegal_wrapper():
32533253
t = _make_illegal_wrapper()
32543254
self.assertRaises(TypeError, t.read)
32553255

3256+
# Issue 31243: calling read() while the return value of decoder's
3257+
# getstate() is invalid should neither crash the interpreter nor
3258+
# raise a SystemError.
3259+
def _make_very_illegal_wrapper(getstate_ret_val):
3260+
class BadDecoder:
3261+
def getstate(self):
3262+
return getstate_ret_val
3263+
def _get_bad_decoder(dummy):
3264+
return BadDecoder()
3265+
quopri = codecs.lookup("quopri")
3266+
with support.swap_attr(quopri, 'incrementaldecoder',
3267+
_get_bad_decoder):
3268+
return _make_illegal_wrapper()
3269+
t = _make_very_illegal_wrapper(42)
3270+
self.assertRaises(TypeError, t.read, 42)
3271+
t = _make_very_illegal_wrapper(())
3272+
self.assertRaises(TypeError, t.read, 42)
3273+
t = _make_very_illegal_wrapper((1, 2))
3274+
self.assertRaises(TypeError, t.read, 42)
3275+
32563276
def _check_create_at_shutdown(self, **kwargs):
32573277
# Issue #20037: creating a TextIOWrapper at shutdown
32583278
# shouldn't crash the interpreter.

‎Misc/NEWS.d/next/Core and Builtins/2017-08-24-13-34-49.bpo-31243.dRJzqR.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix a crash in some methods of `io.TextIOWrapper`, when the decoder's state
2+
is invalid. Patch by Oren Milman.

‎Modules/_io/textio.c

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1513,15 +1513,23 @@ textiowrapper_read_chunk(textio *self, Py_ssize_t size_hint)
15131513
/* Given this, we know there was a valid snapshot point
15141514
* len(dec_buffer) bytes ago with decoder state (b'', dec_flags).
15151515
*/
1516-
if (PyArg_ParseTuple(state, "OO", &dec_buffer, &dec_flags) < 0) {
1516+
if (!PyTuple_Check(state)) {
1517+
PyErr_SetString(PyExc_TypeError,
1518+
"illegal decoder state");
1519+
Py_DECREF(state);
1520+
return -1;
1521+
}
1522+
if (!PyArg_ParseTuple(state,
1523+
"OO;illegal decoder state", &dec_buffer, &dec_flags))
1524+
{
15171525
Py_DECREF(state);
15181526
return -1;
15191527
}
15201528

15211529
if (!PyBytes_Check(dec_buffer)) {
15221530
PyErr_Format(PyExc_TypeError,
1523-
"decoder getstate() should have returned a bytes "
1524-
"object, not '%.200s'",
1531+
"illegal decoder state: the first item should be a "
1532+
"bytes object, not '%.200s'",
15251533
Py_TYPE(dec_buffer)->tp_name);
15261534
Py_DECREF(state);
15271535
return -1;
@@ -2408,8 +2416,8 @@ _io_TextIOWrapper_tell_impl(textio *self)
24082416
} \
24092417
if (!PyBytes_Check(dec_buffer)) { \
24102418
PyErr_Format(PyExc_TypeError, \
2411-
"decoder getstate() should have returned a bytes " \
2412-
"object, not '%.200s'", \
2419+
"illegal decoder state: the first item should be a " \
2420+
"bytes object, not '%.200s'", \
24132421
Py_TYPE(dec_buffer)->tp_name); \
24142422
Py_DECREF(_state); \
24152423
goto fail; \

‎Modules/_testcapimodule.c

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -864,8 +864,9 @@ test_L_code(PyObject *self)
864864
PyTuple_SET_ITEM(tuple, 0, num);
865865

866866
value = -1;
867-
if (PyArg_ParseTuple(tuple, "L:test_L_code", &value) < 0)
867+
if (!PyArg_ParseTuple(tuple, "L:test_L_code", &value)) {
868868
return NULL;
869+
}
869870
if (value != 42)
870871
return raiseTestError("test_L_code",
871872
"L code returned wrong value for long 42");
@@ -878,8 +879,9 @@ test_L_code(PyObject *self)
878879
PyTuple_SET_ITEM(tuple, 0, num);
879880

880881
value = -1;
881-
if (PyArg_ParseTuple(tuple, "L:test_L_code", &value) < 0)
882+
if (!PyArg_ParseTuple(tuple, "L:test_L_code", &value)) {
882883
return NULL;
884+
}
883885
if (value != 42)
884886
return raiseTestError("test_L_code",
885887
"L code returned wrong value for int 42");
@@ -1195,8 +1197,9 @@ test_k_code(PyObject *self)
11951197
PyTuple_SET_ITEM(tuple, 0, num);
11961198

11971199
value = 0;
1198-
if (PyArg_ParseTuple(tuple, "k:test_k_code", &value) < 0)
1200+
if (!PyArg_ParseTuple(tuple, "k:test_k_code", &value)) {
11991201
return NULL;
1202+
}
12001203
if (value != ULONG_MAX)
12011204
return raiseTestError("test_k_code",
12021205
"k code returned wrong value for long 0xFFF...FFF");
@@ -1215,8 +1218,9 @@ test_k_code(PyObject *self)
12151218
PyTuple_SET_ITEM(tuple, 0, num);
12161219

12171220
value = 0;
1218-
if (PyArg_ParseTuple(tuple, "k:test_k_code", &value) < 0)
1221+
if (!PyArg_ParseTuple(tuple, "k:test_k_code", &value)) {
12191222
return NULL;
1223+
}
12201224
if (value != (unsigned long)-0x42)
12211225
return raiseTestError("test_k_code",
12221226
"k code returned wrong value for long -0xFFF..000042");
@@ -1549,11 +1553,13 @@ test_s_code(PyObject *self)
15491553
/* These two blocks used to raise a TypeError:
15501554
* "argument must be string without null bytes, not str"
15511555
*/
1552-
if (PyArg_ParseTuple(tuple, "s:test_s_code1", &value) < 0)
1553-
return NULL;
1556+
if (!PyArg_ParseTuple(tuple, "s:test_s_code1", &value)) {
1557+
return NULL;
1558+
}
15541559

1555-
if (PyArg_ParseTuple(tuple, "z:test_s_code2", &value) < 0)
1556-
return NULL;
1560+
if (!PyArg_ParseTuple(tuple, "z:test_s_code2", &value)) {
1561+
return NULL;
1562+
}
15571563

15581564
Py_DECREF(tuple);
15591565
Py_RETURN_NONE;
@@ -1655,14 +1661,16 @@ test_u_code(PyObject *self)
16551661
PyTuple_SET_ITEM(tuple, 0, obj);
16561662

16571663
value = 0;
1658-
if (PyArg_ParseTuple(tuple, "u:test_u_code", &value) < 0)
1664+
if (!PyArg_ParseTuple(tuple, "u:test_u_code", &value)) {
16591665
return NULL;
1666+
}
16601667
if (value != PyUnicode_AS_UNICODE(obj))
16611668
return raiseTestError("test_u_code",
16621669
"u code returned wrong value for u'test'");
16631670
value = 0;
1664-
if (PyArg_ParseTuple(tuple, "u#:test_u_code", &value, &len) < 0)
1671+
if (!PyArg_ParseTuple(tuple, "u#:test_u_code", &value, &len)) {
16651672
return NULL;
1673+
}
16661674
if (value != PyUnicode_AS_UNICODE(obj) ||
16671675
len != PyUnicode_GET_SIZE(obj))
16681676
return raiseTestError("test_u_code",
@@ -1694,8 +1702,9 @@ test_Z_code(PyObject *self)
16941702
value2 = PyUnicode_AS_UNICODE(obj);
16951703

16961704
/* Test Z for both values */
1697-
if (PyArg_ParseTuple(tuple, "ZZ:test_Z_code", &value1, &value2) < 0)
1705+
if (!PyArg_ParseTuple(tuple, "ZZ:test_Z_code", &value1, &value2)) {
16981706
return NULL;
1707+
}
16991708
if (value1 != PyUnicode_AS_UNICODE(obj))
17001709
return raiseTestError("test_Z_code",
17011710
"Z code returned wrong value for 'test'");
@@ -1709,9 +1718,11 @@ test_Z_code(PyObject *self)
17091718
len2 = -1;
17101719

17111720
/* Test Z# for both values */
1712-
if (PyArg_ParseTuple(tuple, "Z#Z#:test_Z_code", &value1, &len1,
1713-
&value2, &len2) < 0)
1721+
if (!PyArg_ParseTuple(tuple, "Z#Z#:test_Z_code", &value1, &len1,
1722+
&value2, &len2))
1723+
{
17141724
return NULL;
1725+
}
17151726
if (value1 != PyUnicode_AS_UNICODE(obj) ||
17161727
len1 != PyUnicode_GET_SIZE(obj))
17171728
return raiseTestError("test_Z_code",
@@ -2033,17 +2044,19 @@ test_empty_argparse(PyObject *self)
20332044
tuple = PyTuple_New(0);
20342045
if (!tuple)
20352046
return NULL;
2036-
if ((result = PyArg_ParseTuple(tuple, "|:test_empty_argparse")) < 0)
2047+
if (!(result = PyArg_ParseTuple(tuple, "|:test_empty_argparse"))) {
20372048
goto done;
2049+
}
20382050
dict = PyDict_New();
20392051
if (!dict)
20402052
goto done;
20412053
result = PyArg_ParseTupleAndKeywords(tuple, dict, "|:test_empty_argparse", kwlist);
20422054
done:
20432055
Py_DECREF(tuple);
20442056
Py_XDECREF(dict);
2045-
if (result < 0)
2057+
if (!result) {
20462058
return NULL;
2059+
}
20472060
else {
20482061
Py_RETURN_NONE;
20492062
}
@@ -3698,8 +3711,9 @@ test_raise_signal(PyObject* self, PyObject *args)
36983711
{
36993712
int signum, err;
37003713

3701-
if (PyArg_ParseTuple(args, "i:raise_signal", &signum) < 0)
3714+
if (!PyArg_ParseTuple(args, "i:raise_signal", &signum)) {
37023715
return NULL;
3716+
}
37033717

37043718
err = raise(signum);
37053719
if (err)

0 commit comments

Comments
 (0)