Skip to content

Commit 16e7e9b

Browse files
gh-132915: Try to detect a buffer overflow in fcntl() and ioctl()
SystemError is raised when buffer overflow is detected. The stack and memory can already be corrupted, so treat this error as fatal.
1 parent 9f5994b commit 16e7e9b

File tree

2 files changed

+41
-21
lines changed

2 files changed

+41
-21
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:func:`fcntl.fcntl` and :func:`fcntl.ioctl` can now detect a buffer overflow
2+
and raise :exc:`SystemError`. The stack and memory can be corrupted in such
3+
case, so treat this error as fatal.

Modules/fcntlmodule.c

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@
2222
# include <stropts.h> // I_FLUSHBAND
2323
#endif
2424

25+
#define GUARDSZ 16
26+
// NUL followed by random bytes.
27+
static const char guard[GUARDSZ] = "\x00\xfa\x69\xc4\x67\xa3\x6c\x58"
28+
"\x06\x41\x8c\x77\x54\xd6\x51\x6b";
29+
2530
/*[clinic input]
2631
module fcntl
2732
[clinic start generated code]*/
@@ -80,9 +85,10 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg)
8085
return PyLong_FromLong(ret);
8186
}
8287
if (PyUnicode_Check(arg) || PyObject_CheckBuffer(arg)) {
83-
#define FCNTL_BUFSZ 1024
8488
Py_buffer view;
85-
char buf[FCNTL_BUFSZ+1]; /* argument plus NUL byte */
89+
#define FCNTL_BUFSZ 1024
90+
/* argument plus NUL byte plus guard to detect a buffer overflow */
91+
char buf[FCNTL_BUFSZ+GUARDSZ];
8692

8793
if (!PyArg_Parse(arg, "s*", &view)) {
8894
return NULL;
@@ -95,7 +101,7 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg)
95101
return NULL;
96102
}
97103
memcpy(buf, view.buf, len);
98-
buf[len] = '\0';
104+
memcpy(buf + len, guard, GUARDSZ);
99105
PyBuffer_Release(&view);
100106

101107
do {
@@ -106,6 +112,10 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg)
106112
if (ret < 0) {
107113
return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL;
108114
}
115+
if (memcmp(buf + len, guard, GUARDSZ) != 0) {
116+
PyErr_SetString(PyExc_SystemError, "buffer overflow1");
117+
return NULL;
118+
}
109119
return PyBytes_FromStringAndSize(buf, len);
110120
#undef FCNTL_BUFSZ
111121
}
@@ -199,34 +209,37 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg,
199209
if (PyUnicode_Check(arg) || PyObject_CheckBuffer(arg)) {
200210
Py_buffer view;
201211
#define IOCTL_BUFSZ 1024
202-
char buf[IOCTL_BUFSZ+1]; /* argument plus NUL byte */
212+
/* argument plus NUL byte plus guard to detect a buffer overflow */
213+
char buf[IOCTL_BUFSZ+GUARDSZ];
203214
if (mutate_arg && !PyBytes_Check(arg) && !PyUnicode_Check(arg)) {
204215
if (PyObject_GetBuffer(arg, &view, PyBUF_WRITABLE) == 0) {
205-
if (view.len <= IOCTL_BUFSZ) {
206-
memcpy(buf, view.buf, view.len);
207-
buf[view.len] = '\0';
208-
do {
209-
Py_BEGIN_ALLOW_THREADS
210-
ret = ioctl(fd, code, buf);
211-
Py_END_ALLOW_THREADS
212-
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
213-
memcpy(view.buf, buf, view.len);
214-
}
215-
else {
216-
do {
217-
Py_BEGIN_ALLOW_THREADS
218-
ret = ioctl(fd, code, view.buf);
219-
Py_END_ALLOW_THREADS
220-
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
216+
Py_ssize_t len = view.len;
217+
void *ptr = view.buf;
218+
if (len <= IOCTL_BUFSZ) {
219+
memcpy(buf, ptr, len);
220+
memcpy(buf + len, guard, GUARDSZ);
221+
ptr = buf;
221222
}
223+
do {
224+
Py_BEGIN_ALLOW_THREADS
225+
ret = ioctl(fd, code, ptr);
226+
Py_END_ALLOW_THREADS
227+
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
222228
if (ret < 0) {
223229
if (!async_err) {
224230
PyErr_SetFromErrno(PyExc_OSError);
225231
}
226232
PyBuffer_Release(&view);
227233
return NULL;
228234
}
235+
if (ptr == buf) {
236+
memcpy(view.buf, buf, len);
237+
}
229238
PyBuffer_Release(&view);
239+
if (ptr == buf && memcmp(buf + len, guard, GUARDSZ) != 0) {
240+
PyErr_SetString(PyExc_SystemError, "buffer overflow2");
241+
return NULL;
242+
}
230243
return PyLong_FromLong(ret);
231244
}
232245
if (!PyErr_ExceptionMatches(PyExc_BufferError)) {
@@ -246,7 +259,7 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg,
246259
return NULL;
247260
}
248261
memcpy(buf, view.buf, len);
249-
buf[len] = '\0';
262+
memcpy(buf + len, guard, GUARDSZ);
250263
PyBuffer_Release(&view);
251264

252265
do {
@@ -257,6 +270,10 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg,
257270
if (ret < 0) {
258271
return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL;
259272
}
273+
if (memcmp(buf + len, guard, GUARDSZ) != 0) {
274+
PyErr_SetString(PyExc_SystemError, "buffer overflow3");
275+
return NULL;
276+
}
260277
return PyBytes_FromStringAndSize(buf, len);
261278
#undef IOCTL_BUFSZ
262279
}

0 commit comments

Comments
 (0)