Skip to content

Commit 0f0a362

Browse files
cmaloneyashm-devvstinner
authored
gh-140607: Validate returned byte count in RawIOBase.read (#140611)
While `RawIOBase.readinto` should return a count of bytes between 0 and the length of the given buffer, it is not required to. Add validation inside RawIOBase.read() that the returned byte count is valid. Co-authored-by: Shamil <[email protected]> Co-authored-by: Victor Stinner <[email protected]>
1 parent 313145e commit 0f0a362

File tree

4 files changed

+30
-3
lines changed

4 files changed

+30
-3
lines changed

Lib/_pyio.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,8 @@ def read(self, size=-1):
617617
n = self.readinto(b)
618618
if n is None:
619619
return None
620+
if n < 0 or n > len(b):
621+
raise ValueError(f"readinto returned {n} outside buffer size {len(b)}")
620622
del b[n:]
621623
return bytes(b)
622624

Lib/test/test_io/test_general.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,22 @@ def test_RawIOBase_read(self):
592592
self.assertEqual(rawio.read(2), None)
593593
self.assertEqual(rawio.read(2), b"")
594594

595+
def test_RawIOBase_read_bounds_checking(self):
596+
# Make sure a `.readinto` call which returns a value outside
597+
# (0, len(buffer)) raises.
598+
class Misbehaved(self.io.RawIOBase):
599+
def __init__(self, readinto_return) -> None:
600+
self._readinto_return = readinto_return
601+
def readinto(self, b):
602+
return self._readinto_return
603+
604+
with self.assertRaises(ValueError) as cm:
605+
Misbehaved(2).read(1)
606+
self.assertEqual(str(cm.exception), "readinto returned 2 outside buffer size 1")
607+
for bad_size in (2147483647, sys.maxsize, -1, -1000):
608+
with self.assertRaises(ValueError):
609+
Misbehaved(bad_size).read()
610+
595611
def test_types_have_dict(self):
596612
test = (
597613
self.IOBase(),
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Inside :meth:`io.RawIOBase.read`, validate that the count of bytes returned by
2+
:meth:`io.RawIOBase.readinto` is valid (inside the provided buffer).

Modules/_io/iobase.c

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -939,14 +939,21 @@ _io__RawIOBase_read_impl(PyObject *self, Py_ssize_t n)
939939
return res;
940940
}
941941

942-
n = PyNumber_AsSsize_t(res, PyExc_ValueError);
942+
Py_ssize_t bytes_filled = PyNumber_AsSsize_t(res, PyExc_ValueError);
943943
Py_DECREF(res);
944-
if (n == -1 && PyErr_Occurred()) {
944+
if (bytes_filled == -1 && PyErr_Occurred()) {
945945
Py_DECREF(b);
946946
return NULL;
947947
}
948+
if (bytes_filled < 0 || bytes_filled > n) {
949+
Py_DECREF(b);
950+
PyErr_Format(PyExc_ValueError,
951+
"readinto returned %zd outside buffer size %zd",
952+
bytes_filled, n);
953+
return NULL;
954+
}
948955

949-
res = PyBytes_FromStringAndSize(PyByteArray_AsString(b), n);
956+
res = PyBytes_FromStringAndSize(PyByteArray_AsString(b), bytes_filled);
950957
Py_DECREF(b);
951958
return res;
952959
}

0 commit comments

Comments
 (0)