Skip to content

Commit

Permalink
pythongh-95041: Fix several minor issues in syslog.openlog() (pythonG…
Browse files Browse the repository at this point in the history
…H-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().
(cherry picked from commit 68c555a)

Co-authored-by: Serhiy Storchaka <[email protected]>
  • Loading branch information
miss-islington and serhiy-storchaka authored Jul 26, 2022
1 parent 0418d9f commit dd0c1a3
Showing 1 changed file with 33 additions and 24 deletions.
57 changes: 33 additions & 24 deletions Modules/syslogmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}


Expand All @@ -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);
}

/* 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();
else {
/* get sys.argv[0] or NULL if we can't for some reason */
ident = 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;
}
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -355,4 +364,4 @@ PyMODINIT_FUNC
PyInit_syslog(void)
{
return PyModuleDef_Init(&syslogmodule);
}
}

0 comments on commit dd0c1a3

Please sign in to comment.