gh-95041: Fix several minor issues in syslog.openlog() (GH-95058)

* syslog_get_argv() swallows exceptions, but not in all cases.
* if ident is non UTF-8 encodable, syslog.openlog() fails after setting the
  global reference to ident. Now the C string saved internally in the previous
  call to openlog() points to the freed memory.
* PySys_Audit() can crash if ident is NULL.
* There may be a race condition with syslog.syslog(), because the global
  reference to ident is decrefed before setting the new value.
* Possible use of freed memory if syslog.openlog() is called while
  the GIL is released in syslog.syslog().
This commit is contained in:
Serhiy Storchaka 2022-07-26 09:12:10 +03:00 committed by GitHub
parent 2ef73be891
commit 68c555a50a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -87,6 +87,10 @@ syslog_get_argv(void)
}
scriptobj = PyList_GetItem(argv, 0);
if (scriptobj == NULL) {
PyErr_Clear();
return NULL;
}
if (!PyUnicode_Check(scriptobj)) {
return(NULL);
}
@ -96,16 +100,16 @@ syslog_get_argv(void)
}
slash = PyUnicode_FindChar(scriptobj, SEP, 0, scriptlen, -1);
if (slash == -2)
if (slash == -2) {
PyErr_Clear();
return NULL;
}
if (slash != -1) {
return PyUnicode_Substring(scriptobj, slash + 1, scriptlen);
} else {
Py_INCREF(scriptobj);
return(scriptobj);
}
return(NULL);
}
@ -114,42 +118,41 @@ syslog_openlog(PyObject * self, PyObject * args, PyObject *kwds)
{
long logopt = 0;
long facility = LOG_USER;
PyObject *new_S_ident_o = NULL;
PyObject *ident = NULL;
static char *keywords[] = {"ident", "logoption", "facility", 0};
const char *ident = NULL;
const char *ident_str = NULL;
if (!PyArg_ParseTupleAndKeywords(args, kwds,
"|Ull:openlog", keywords, &new_S_ident_o, &logopt, &facility))
"|Ull:openlog", keywords, &ident, &logopt, &facility))
return NULL;
if (new_S_ident_o) {
Py_INCREF(new_S_ident_o);
if (ident) {
Py_INCREF(ident);
}
else {
/* get sys.argv[0] or NULL if we can't for some reason */
ident = syslog_get_argv();
}
/* get sys.argv[0] or NULL if we can't for some reason */
if (!new_S_ident_o) {
new_S_ident_o = syslog_get_argv();
}
Py_XDECREF(S_ident_o);
S_ident_o = new_S_ident_o;
/* At this point, S_ident_o should be INCREF()ed. openlog(3) does not
* make a copy, and syslog(3) later uses it. We can't garbagecollect it
/* At this point, ident should be INCREF()ed. openlog(3) does not
* make a copy, and syslog(3) later uses it. We can't garbagecollect it.
* If NULL, just let openlog figure it out (probably using C argv[0]).
*/
if (S_ident_o) {
ident = PyUnicode_AsUTF8(S_ident_o);
if (ident == NULL)
if (ident) {
ident_str = PyUnicode_AsUTF8(ident);
if (ident_str == NULL) {
Py_DECREF(ident);
return NULL;
}
}
if (PySys_Audit("syslog.openlog", "sll", ident, logopt, facility) < 0) {
if (PySys_Audit("syslog.openlog", "Oll", ident ? ident : Py_None, logopt, facility) < 0) {
Py_DECREF(ident);
return NULL;
}
openlog(ident, logopt, facility);
openlog(ident_str, logopt, facility);
S_log_open = 1;
Py_XSETREF(S_ident_o, ident);
Py_RETURN_NONE;
}
@ -193,9 +196,15 @@ syslog_syslog(PyObject * self, PyObject * args)
}
}
/* Incref ident, because it can be decrefed if syslog.openlog() is
* called when the GIL is released.
*/
PyObject *ident = S_ident_o;
Py_XINCREF(ident);
Py_BEGIN_ALLOW_THREADS;
syslog(priority, "%s", message);
Py_END_ALLOW_THREADS;
Py_XDECREF(ident);
Py_RETURN_NONE;
}
@ -355,4 +364,4 @@ PyMODINIT_FUNC
PyInit_syslog(void)
{
return PyModuleDef_Init(&syslogmodule);
}
}