From 88bb327915c44fd90de1c519ec8c0d6b579eeacf Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 10 Apr 2023 21:07:43 +0200 Subject: [PATCH 1/5] gh-83004: Harden msvcrt further --- PC/msvcrtmodule.c | 118 ++++++++++++++++++++-------------------------- 1 file changed, 52 insertions(+), 66 deletions(-) diff --git a/PC/msvcrtmodule.c b/PC/msvcrtmodule.c index 6e8b423c3839a9..d690e62e9971ff 100644 --- a/PC/msvcrtmodule.c +++ b/PC/msvcrtmodule.c @@ -577,92 +577,80 @@ static struct PyModuleDef msvcrtmodule = { NULL }; -static void -insertint(PyObject *d, char *name, int value) -{ - PyObject *v = PyLong_FromLong((long) value); - if (v == NULL) { - /* Don't bother reporting this error */ - PyErr_Clear(); - } - else { - PyDict_SetItemString(d, name, v); - Py_DECREF(v); - } -} - -static void -insertptr(PyObject *d, char *name, void *value) -{ - PyObject *v = PyLong_FromVoidPtr(value); - if (v == NULL) { - /* Don't bother reporting this error */ - PyErr_Clear(); - } - else { - PyDict_SetItemString(d, name, v); - Py_DECREF(v); - } -} +#define INSERTINT(MOD, NAME, VAL) do { \ + if (PyModule_AddIntConstant(MOD, NAME, VAL) < 0) { \ + goto error; \ + } \ +} while (0) + +#define INSERTPTR(MOD, NAME, PTR) do { \ + PyObject *v = PyLong_FromVoidPtr(PTR); \ + if (v == NULL) { \ + goto error; \ + } \ + int rc = PyModule_AddObjectRef(MOD, NAME, v); \ + Py_DECREF(v); \ + if (rc < 0) { \ + goto error; \ + } \ +} while (0) + +#define INSERTSTR(MOD, NAME, CONST) do { \ + if (PyModule_AddStringConstant(MOD, NAME, CONST) < 0) { \ + goto error; \ + } \ +} while (0) PyMODINIT_FUNC PyInit_msvcrt(void) { - int st; PyObject *m = PyModule_Create(&msvcrtmodule); if (m == NULL) { return NULL; } - PyObject *d = PyModule_GetDict(m); // Borrowed ref. /* constants for the locking() function's mode argument */ - insertint(d, "LK_LOCK", _LK_LOCK); - insertint(d, "LK_NBLCK", _LK_NBLCK); - insertint(d, "LK_NBRLCK", _LK_NBRLCK); - insertint(d, "LK_RLCK", _LK_RLCK); - insertint(d, "LK_UNLCK", _LK_UNLCK); + INSERTINT(m, "LK_LOCK", _LK_LOCK); + INSERTINT(m, "LK_NBLCK", _LK_NBLCK); + INSERTINT(m, "LK_NBRLCK", _LK_NBRLCK); + INSERTINT(m, "LK_RLCK", _LK_RLCK); + INSERTINT(m, "LK_UNLCK", _LK_UNLCK); #ifdef MS_WINDOWS_DESKTOP - insertint(d, "SEM_FAILCRITICALERRORS", SEM_FAILCRITICALERRORS); - insertint(d, "SEM_NOALIGNMENTFAULTEXCEPT", SEM_NOALIGNMENTFAULTEXCEPT); - insertint(d, "SEM_NOGPFAULTERRORBOX", SEM_NOGPFAULTERRORBOX); - insertint(d, "SEM_NOOPENFILEERRORBOX", SEM_NOOPENFILEERRORBOX); + INSERTINT(m, "SEM_FAILCRITICALERRORS", SEM_FAILCRITICALERRORS); + INSERTINT(m, "SEM_NOALIGNMENTFAULTEXCEPT", SEM_NOALIGNMENTFAULTEXCEPT); + INSERTINT(m, "SEM_NOGPFAULTERRORBOX", SEM_NOGPFAULTERRORBOX); + INSERTINT(m, "SEM_NOOPENFILEERRORBOX", SEM_NOOPENFILEERRORBOX); #endif #ifdef _DEBUG - insertint(d, "CRT_WARN", _CRT_WARN); - insertint(d, "CRT_ERROR", _CRT_ERROR); - insertint(d, "CRT_ASSERT", _CRT_ASSERT); - insertint(d, "CRTDBG_MODE_DEBUG", _CRTDBG_MODE_DEBUG); - insertint(d, "CRTDBG_MODE_FILE", _CRTDBG_MODE_FILE); - insertint(d, "CRTDBG_MODE_WNDW", _CRTDBG_MODE_WNDW); - insertint(d, "CRTDBG_REPORT_MODE", _CRTDBG_REPORT_MODE); - insertptr(d, "CRTDBG_FILE_STDERR", _CRTDBG_FILE_STDERR); - insertptr(d, "CRTDBG_FILE_STDOUT", _CRTDBG_FILE_STDOUT); - insertptr(d, "CRTDBG_REPORT_FILE", _CRTDBG_REPORT_FILE); + INSERTINT(m, "CRT_WARN", _CRT_WARN); + INSERTINT(m, "CRT_ERROR", _CRT_ERROR); + INSERTINT(m, "CRT_ASSERT", _CRT_ASSERT); + INSERTINT(m, "CRTDBG_MODE_DEBUG", _CRTDBG_MODE_DEBUG); + INSERTINT(m, "CRTDBG_MODE_FILE", _CRTDBG_MODE_FILE); + INSERTINT(m, "CRTDBG_MODE_WNDW", _CRTDBG_MODE_WNDW); + INSERTINT(m, "CRTDBG_REPORT_MODE", _CRTDBG_REPORT_MODE); + INSERTPTR(m, "CRTDBG_FILE_STDERR", _CRTDBG_FILE_STDERR); + INSERTPTR(m, "CRTDBG_FILE_STDOUT", _CRTDBG_FILE_STDOUT); + INSERTPTR(m, "CRTDBG_REPORT_FILE", _CRTDBG_REPORT_FILE); #endif +#undef INSERTINT +#undef INSERTPTR + /* constants for the crt versions */ #ifdef _VC_ASSEMBLY_PUBLICKEYTOKEN - st = PyModule_AddStringConstant(m, "VC_ASSEMBLY_PUBLICKEYTOKEN", - _VC_ASSEMBLY_PUBLICKEYTOKEN); - if (st < 0) { - goto error; - } + INSERTSTR(m, "VC_ASSEMBLY_PUBLICKEYTOKEN", _VC_ASSEMBLY_PUBLICKEYTOKEN); #endif #ifdef _CRT_ASSEMBLY_VERSION - st = PyModule_AddStringConstant(m, "CRT_ASSEMBLY_VERSION", - _CRT_ASSEMBLY_VERSION); - if (st < 0) { - goto error; - } + INSERTSTR(m, "CRT_ASSEMBLY_VERSION", _CRT_ASSEMBLY_VERSION); #endif #ifdef __LIBRARIES_ASSEMBLY_NAME_PREFIX - st = PyModule_AddStringConstant(m, "LIBRARIES_ASSEMBLY_NAME_PREFIX", - __LIBRARIES_ASSEMBLY_NAME_PREFIX); - if (st < 0) { - goto error; - } + INSERTSTR(m, "LIBRARIES_ASSEMBLY_NAME_PREFIX", + __LIBRARIES_ASSEMBLY_NAME_PREFIX); #endif +#undef INSERTSTR + /* constants for the 2010 crt versions */ #if defined(_VC_CRT_MAJOR_VERSION) && defined (_VC_CRT_MINOR_VERSION) && defined(_VC_CRT_BUILD_VERSION) && defined(_VC_CRT_RBUILD_VERSION) PyObject *version = PyUnicode_FromFormat("%d.%d.%d.%d", @@ -673,14 +661,12 @@ PyInit_msvcrt(void) if (version == NULL) { goto error; } - st = PyModule_AddObjectRef(m, "CRT_ASSEMBLY_VERSION", version); + int st = PyModule_AddObjectRef(m, "CRT_ASSEMBLY_VERSION", version); Py_DECREF(version); if (st < 0) { goto error; } #endif - /* make compiler warning quiet if st is unused */ - (void)st; return m; From 6c1f145f471442596e4aecf103a23823f5e06e09 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 10 Apr 2023 22:16:02 +0200 Subject: [PATCH 2/5] Make the diff slightly nicer --- PC/msvcrtmodule.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/PC/msvcrtmodule.c b/PC/msvcrtmodule.c index d690e62e9971ff..b257e643ce203e 100644 --- a/PC/msvcrtmodule.c +++ b/PC/msvcrtmodule.c @@ -577,22 +577,28 @@ static struct PyModuleDef msvcrtmodule = { NULL }; +static int +insertptr(PyObject *mod, char *name, void *value) +{ + PyObject *v = PyLong_FromVoidPtr(value); + if (v == NULL) { + return -1; + } + int rc = PyModule_AddObjectRef(mod, name, v); + Py_DECREF(v); + return rc; +} + #define INSERTINT(MOD, NAME, VAL) do { \ if (PyModule_AddIntConstant(MOD, NAME, VAL) < 0) { \ goto error; \ } \ } while (0) -#define INSERTPTR(MOD, NAME, PTR) do { \ - PyObject *v = PyLong_FromVoidPtr(PTR); \ - if (v == NULL) { \ - goto error; \ - } \ - int rc = PyModule_AddObjectRef(MOD, NAME, v); \ - Py_DECREF(v); \ - if (rc < 0) { \ - goto error; \ - } \ +#define INSERTPTR(MOD, NAME, PTR) do { \ + if (insertptr(MOD, NAME, PTR) < 0) { \ + goto error; \ + } \ } while (0) #define INSERTSTR(MOD, NAME, CONST) do { \ From 9ff5c3f3f4c22b81f4ff323f76607840eb70e971 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Tue, 18 Apr 2023 16:21:54 +0530 Subject: [PATCH 3/5] Update PC/msvcrtmodule.c --- PC/msvcrtmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PC/msvcrtmodule.c b/PC/msvcrtmodule.c index d488658d316ae3..834ba3e2e6a2ad 100644 --- a/PC/msvcrtmodule.c +++ b/PC/msvcrtmodule.c @@ -651,7 +651,7 @@ exec_module(PyObject* m) if (version == NULL) { return -1; } - int st = PyModule_AddObjectRef(m, "CRT_ASSEMBLY_VERSION", version); + st = PyModule_AddObjectRef(m, "CRT_ASSEMBLY_VERSION", version); Py_DECREF(version); if (st < 0) { return -1; From b058d0915992f59158e5c5417aa9aa1b9e60fc33 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 18 Apr 2023 13:12:00 -0600 Subject: [PATCH 4/5] Intermingled declaration --- PC/msvcrtmodule.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/PC/msvcrtmodule.c b/PC/msvcrtmodule.c index 834ba3e2e6a2ad..7ccb7772539561 100644 --- a/PC/msvcrtmodule.c +++ b/PC/msvcrtmodule.c @@ -597,8 +597,6 @@ insertptr(PyObject *mod, char *name, void *value) static int exec_module(PyObject* m) { - int st; - /* constants for the locking() function's mode argument */ INSERTINT(m, "LK_LOCK", _LK_LOCK); INSERTINT(m, "LK_NBLCK", _LK_NBLCK); @@ -651,7 +649,7 @@ exec_module(PyObject* m) if (version == NULL) { return -1; } - st = PyModule_AddObjectRef(m, "CRT_ASSEMBLY_VERSION", version); + int st = PyModule_AddObjectRef(m, "CRT_ASSEMBLY_VERSION", version); Py_DECREF(version); if (st < 0) { return -1; From 1f2162be5031851504b0c21fe7e6960f44aa030e Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 18 Apr 2023 13:12:39 -0600 Subject: [PATCH 5/5] Adress review: Return -1 iso. goto error --- PC/msvcrtmodule.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/PC/msvcrtmodule.c b/PC/msvcrtmodule.c index 7ccb7772539561..090254befc934d 100644 --- a/PC/msvcrtmodule.c +++ b/PC/msvcrtmodule.c @@ -578,19 +578,19 @@ insertptr(PyObject *mod, char *name, void *value) #define INSERTINT(MOD, NAME, VAL) do { \ if (PyModule_AddIntConstant(MOD, NAME, VAL) < 0) { \ - goto error; \ + return -1; \ } \ } while (0) #define INSERTPTR(MOD, NAME, PTR) do { \ if (insertptr(MOD, NAME, PTR) < 0) { \ - goto error; \ + return -1; \ } \ } while (0) #define INSERTSTR(MOD, NAME, CONST) do { \ if (PyModule_AddStringConstant(MOD, NAME, CONST) < 0) { \ - goto error; \ + return -1; \ } \ } while (0)