Skip to content

Commit 6627b40

Browse files
committed
gh-156939: Fix buffer overflow in fcntl
Allocate one extra "canary byte" to detect buffer overflow. Previously, the canary byte (NUL byte) was written after the allocated byte which would lead to buffer overflow if the bytes writer uses the small buffer.
1 parent 1f25c33 commit 6627b40

1 file changed

Lines changed: 11 additions & 6 deletions

File tree

‎Modules/fcntlmodule.c‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#define GUARDSZ 8
2525
// NUL followed by random bytes.
2626
static const char guard[GUARDSZ] _Py_NONSTRING = "\x00\xfa\x69\xc4\x67\xa3\x6c\x58";
27+
const char CANARY_BYTE = 0xdd;
2728

2829
/*[clinic input]
2930
module fcntl
@@ -121,13 +122,14 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg)
121122
return PyBytes_FromStringAndSize(buf, len);
122123
}
123124
else {
124-
PyBytesWriter *writer = PyBytesWriter_Create(len);
125+
PyBytesWriter *writer = PyBytesWriter_Create(len + 1);
125126
if (writer == NULL) {
126127
PyBuffer_Release(&view);
127128
return NULL;
128129
}
129130
char *ptr = PyBytesWriter_GetData(writer);
130131
memcpy(ptr, view.buf, len);
132+
ptr[len] = CANARY_BYTE;
131133
PyBuffer_Release(&view);
132134

133135
do {
@@ -142,7 +144,7 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg)
142144
PyBytesWriter_Discard(writer);
143145
return NULL;
144146
}
145-
if (ptr[len] != '\0') {
147+
if (ptr[len] != CANARY_BYTE) {
146148
PyErr_SetString(PyExc_SystemError,
147149
"Memory corruption in fcntl() due to "
148150
"buffer overflow. "
@@ -151,7 +153,8 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg)
151153
PyBytesWriter_Discard(writer);
152154
return NULL;
153155
}
154-
return PyBytesWriter_Finish(writer);
156+
// Truncate the last byte (canary byte)
157+
return PyBytesWriter_FinishWithSize(writer, len);
155158
}
156159
#undef FCNTL_BUFSZ
157160
}
@@ -316,13 +319,14 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg,
316319
return PyBytes_FromStringAndSize(buf, len);
317320
}
318321
else {
319-
PyBytesWriter *writer = PyBytesWriter_Create(len);
322+
PyBytesWriter *writer = PyBytesWriter_Create(len + 1);
320323
if (writer == NULL) {
321324
PyBuffer_Release(&view);
322325
return NULL;
323326
}
324327
char *ptr = PyBytesWriter_GetData(writer);
325328
memcpy(ptr, view.buf, len);
329+
ptr[len] = CANARY_BYTE;
326330
PyBuffer_Release(&view);
327331

328332
do {
@@ -337,7 +341,7 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg,
337341
PyBytesWriter_Discard(writer);
338342
return NULL;
339343
}
340-
if (ptr[len] != '\0') {
344+
if (ptr[len] != CANARY_BYTE) {
341345
PyErr_SetString(PyExc_SystemError,
342346
"Memory corruption in ioctl() due to "
343347
"buffer overflow. "
@@ -346,7 +350,8 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg,
346350
PyBytesWriter_Discard(writer);
347351
return NULL;
348352
}
349-
return PyBytesWriter_Finish(writer);
353+
// Truncate the last byte (canary byte)
354+
return PyBytesWriter_FinishWithSize(writer, len);
350355
}
351356
#undef IOCTL_BUFSZ
352357
}

0 commit comments

Comments
 (0)