From 6627b40818049b231bd4f5b99fe7217d4bad95c1 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 7 Sep 2026 20:14:47 +0200 Subject: [PATCH 1/4] 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. --- Modules/fcntlmodule.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/Modules/fcntlmodule.c b/Modules/fcntlmodule.c index e6a40ffc5a26144..f9742d93a0ebeba 100644 --- a/Modules/fcntlmodule.c +++ b/Modules/fcntlmodule.c @@ -24,6 +24,7 @@ #define GUARDSZ 8 // NUL followed by random bytes. static const char guard[GUARDSZ] _Py_NONSTRING = "\x00\xfa\x69\xc4\x67\xa3\x6c\x58"; +const char CANARY_BYTE = 0xdd; /*[clinic input] module fcntl @@ -121,13 +122,14 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg) return PyBytes_FromStringAndSize(buf, len); } else { - PyBytesWriter *writer = PyBytesWriter_Create(len); + PyBytesWriter *writer = PyBytesWriter_Create(len + 1); if (writer == NULL) { PyBuffer_Release(&view); return NULL; } char *ptr = PyBytesWriter_GetData(writer); memcpy(ptr, view.buf, len); + ptr[len] = CANARY_BYTE; PyBuffer_Release(&view); do { @@ -142,7 +144,7 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg) PyBytesWriter_Discard(writer); return NULL; } - if (ptr[len] != '\0') { + if (ptr[len] != CANARY_BYTE) { PyErr_SetString(PyExc_SystemError, "Memory corruption in fcntl() due to " "buffer overflow. " @@ -151,7 +153,8 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg) PyBytesWriter_Discard(writer); return NULL; } - return PyBytesWriter_Finish(writer); + // Truncate the last byte (canary byte) + return PyBytesWriter_FinishWithSize(writer, len); } #undef FCNTL_BUFSZ } @@ -316,13 +319,14 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg, return PyBytes_FromStringAndSize(buf, len); } else { - PyBytesWriter *writer = PyBytesWriter_Create(len); + PyBytesWriter *writer = PyBytesWriter_Create(len + 1); if (writer == NULL) { PyBuffer_Release(&view); return NULL; } char *ptr = PyBytesWriter_GetData(writer); memcpy(ptr, view.buf, len); + ptr[len] = CANARY_BYTE; PyBuffer_Release(&view); do { @@ -337,7 +341,7 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg, PyBytesWriter_Discard(writer); return NULL; } - if (ptr[len] != '\0') { + if (ptr[len] != CANARY_BYTE) { PyErr_SetString(PyExc_SystemError, "Memory corruption in ioctl() due to " "buffer overflow. " @@ -346,7 +350,8 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg, PyBytesWriter_Discard(writer); return NULL; } - return PyBytesWriter_Finish(writer); + // Truncate the last byte (canary byte) + return PyBytesWriter_FinishWithSize(writer, len); } #undef IOCTL_BUFSZ } From d90a2531f0c32e1bd6105c25ce5b46af993be7f6 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 7 Sep 2026 21:39:22 +0200 Subject: [PATCH 2/4] Replace canary byte with guard bytes --- Modules/fcntlmodule.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/Modules/fcntlmodule.c b/Modules/fcntlmodule.c index f9742d93a0ebeba..f7c46367db6ecca 100644 --- a/Modules/fcntlmodule.c +++ b/Modules/fcntlmodule.c @@ -24,7 +24,6 @@ #define GUARDSZ 8 // NUL followed by random bytes. static const char guard[GUARDSZ] _Py_NONSTRING = "\x00\xfa\x69\xc4\x67\xa3\x6c\x58"; -const char CANARY_BYTE = 0xdd; /*[clinic input] module fcntl @@ -122,14 +121,14 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg) return PyBytes_FromStringAndSize(buf, len); } else { - PyBytesWriter *writer = PyBytesWriter_Create(len + 1); + PyBytesWriter *writer = PyBytesWriter_Create(len + GUARDSZ); if (writer == NULL) { PyBuffer_Release(&view); return NULL; } char *ptr = PyBytesWriter_GetData(writer); memcpy(ptr, view.buf, len); - ptr[len] = CANARY_BYTE; + memcpy(ptr + len, guard, GUARDSZ); PyBuffer_Release(&view); do { @@ -144,7 +143,7 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg) PyBytesWriter_Discard(writer); return NULL; } - if (ptr[len] != CANARY_BYTE) { + if (memcmp(ptr + len, guard, GUARDSZ) != 0) { PyErr_SetString(PyExc_SystemError, "Memory corruption in fcntl() due to " "buffer overflow. " @@ -153,7 +152,7 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg) PyBytesWriter_Discard(writer); return NULL; } - // Truncate the last byte (canary byte) + // Truncate the last bytes (guard) return PyBytesWriter_FinishWithSize(writer, len); } #undef FCNTL_BUFSZ @@ -319,14 +318,14 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg, return PyBytes_FromStringAndSize(buf, len); } else { - PyBytesWriter *writer = PyBytesWriter_Create(len + 1); + PyBytesWriter *writer = PyBytesWriter_Create(len + GUARDSZ); if (writer == NULL) { PyBuffer_Release(&view); return NULL; } char *ptr = PyBytesWriter_GetData(writer); memcpy(ptr, view.buf, len); - ptr[len] = CANARY_BYTE; + memcpy(buf + len, guard, GUARDSZ); PyBuffer_Release(&view); do { @@ -341,7 +340,7 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg, PyBytesWriter_Discard(writer); return NULL; } - if (ptr[len] != CANARY_BYTE) { + if (memcmp(ptr + len, guard, GUARDSZ) != 0) { PyErr_SetString(PyExc_SystemError, "Memory corruption in ioctl() due to " "buffer overflow. " @@ -350,7 +349,7 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg, PyBytesWriter_Discard(writer); return NULL; } - // Truncate the last byte (canary byte) + // Truncate the last bytes (guard) return PyBytesWriter_FinishWithSize(writer, len); } #undef IOCTL_BUFSZ From db0582809df95668af6dee993aea78e00951841d Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 7 Sep 2026 21:46:28 +0200 Subject: [PATCH 3/4] Update comment --- Modules/fcntlmodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/fcntlmodule.c b/Modules/fcntlmodule.c index f7c46367db6ecca..b8d8a23c1792fe9 100644 --- a/Modules/fcntlmodule.c +++ b/Modules/fcntlmodule.c @@ -152,7 +152,7 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg) PyBytesWriter_Discard(writer); return NULL; } - // Truncate the last bytes (guard) + // Truncate the trailing guard bytes return PyBytesWriter_FinishWithSize(writer, len); } #undef FCNTL_BUFSZ @@ -349,7 +349,7 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg, PyBytesWriter_Discard(writer); return NULL; } - // Truncate the last bytes (guard) + // Truncate the trailing guard bytes return PyBytesWriter_FinishWithSize(writer, len); } #undef IOCTL_BUFSZ From c303c1e050da6042cdf4944e1d405a719f34777c Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 7 Sep 2026 21:58:49 +0200 Subject: [PATCH 4/4] Fix typo --- Modules/fcntlmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/fcntlmodule.c b/Modules/fcntlmodule.c index b8d8a23c1792fe9..5dd3df9bb408f0c 100644 --- a/Modules/fcntlmodule.c +++ b/Modules/fcntlmodule.c @@ -325,7 +325,7 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg, } char *ptr = PyBytesWriter_GetData(writer); memcpy(ptr, view.buf, len); - memcpy(buf + len, guard, GUARDSZ); + memcpy(ptr + len, guard, GUARDSZ); PyBuffer_Release(&view); do {