Skip to content

Commit dd0c1a3

Browse files
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(). (cherry picked from commit 68c555a) Co-authored-by: Serhiy Storchaka <[email protected]>
1 parent 0418d9f commit dd0c1a3

File tree

1 file changed

+33
-24
lines changed

1 file changed

+33
-24
lines changed

Modules/syslogmodule.c

+33-24
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,10 @@ syslog_get_argv(void)
8787
}
8888

8989
scriptobj = PyList_GetItem(argv, 0);
90+
if (scriptobj == NULL) {
91+
PyErr_Clear();
92+
return NULL;
93+
}
9094
if (!PyUnicode_Check(scriptobj)) {
9195
return(NULL);
9296
}
@@ -96,16 +100,16 @@ syslog_get_argv(void)
96100
}
97101

98102
slash = PyUnicode_FindChar(scriptobj, SEP, 0, scriptlen, -1);
99-
if (slash == -2)
103+
if (slash == -2) {
104+
PyErr_Clear();
100105
return NULL;
106+
}
101107
if (slash != -1) {
102108
return PyUnicode_Substring(scriptobj, slash + 1, scriptlen);
103109
} else {
104110
Py_INCREF(scriptobj);
105111
return(scriptobj);
106112
}
107-
108-
return(NULL);
109113
}
110114

111115

@@ -114,42 +118,41 @@ syslog_openlog(PyObject * self, PyObject * args, PyObject *kwds)
114118
{
115119
long logopt = 0;
116120
long facility = LOG_USER;
117-
PyObject *new_S_ident_o = NULL;
121+
PyObject *ident = NULL;
118122
static char *keywords[] = {"ident", "logoption", "facility", 0};
119-
const char *ident = NULL;
123+
const char *ident_str = NULL;
120124

121125
if (!PyArg_ParseTupleAndKeywords(args, kwds,
122-
"|Ull:openlog", keywords, &new_S_ident_o, &logopt, &facility))
126+
"|Ull:openlog", keywords, &ident, &logopt, &facility))
123127
return NULL;
124128

125-
if (new_S_ident_o) {
126-
Py_INCREF(new_S_ident_o);
129+
if (ident) {
130+
Py_INCREF(ident);
127131
}
128-
129-
/* get sys.argv[0] or NULL if we can't for some reason */
130-
if (!new_S_ident_o) {
131-
new_S_ident_o = syslog_get_argv();
132+
else {
133+
/* get sys.argv[0] or NULL if we can't for some reason */
134+
ident = syslog_get_argv();
132135
}
133136

134-
Py_XDECREF(S_ident_o);
135-
S_ident_o = new_S_ident_o;
136-
137-
/* At this point, S_ident_o should be INCREF()ed. openlog(3) does not
138-
* make a copy, and syslog(3) later uses it. We can't garbagecollect it
137+
/* At this point, ident should be INCREF()ed. openlog(3) does not
138+
* make a copy, and syslog(3) later uses it. We can't garbagecollect it.
139139
* If NULL, just let openlog figure it out (probably using C argv[0]).
140140
*/
141-
if (S_ident_o) {
142-
ident = PyUnicode_AsUTF8(S_ident_o);
143-
if (ident == NULL)
141+
if (ident) {
142+
ident_str = PyUnicode_AsUTF8(ident);
143+
if (ident_str == NULL) {
144+
Py_DECREF(ident);
144145
return NULL;
146+
}
145147
}
146-
147-
if (PySys_Audit("syslog.openlog", "sll", ident, logopt, facility) < 0) {
148+
if (PySys_Audit("syslog.openlog", "Oll", ident ? ident : Py_None, logopt, facility) < 0) {
149+
Py_DECREF(ident);
148150
return NULL;
149151
}
150152

151-
openlog(ident, logopt, facility);
153+
openlog(ident_str, logopt, facility);
152154
S_log_open = 1;
155+
Py_XSETREF(S_ident_o, ident);
153156

154157
Py_RETURN_NONE;
155158
}
@@ -193,9 +196,15 @@ syslog_syslog(PyObject * self, PyObject * args)
193196
}
194197
}
195198

199+
/* Incref ident, because it can be decrefed if syslog.openlog() is
200+
* called when the GIL is released.
201+
*/
202+
PyObject *ident = S_ident_o;
203+
Py_XINCREF(ident);
196204
Py_BEGIN_ALLOW_THREADS;
197205
syslog(priority, "%s", message);
198206
Py_END_ALLOW_THREADS;
207+
Py_XDECREF(ident);
199208
Py_RETURN_NONE;
200209
}
201210

@@ -355,4 +364,4 @@ PyMODINIT_FUNC
355364
PyInit_syslog(void)
356365
{
357366
return PyModuleDef_Init(&syslogmodule);
358-
}
367+
}

0 commit comments

Comments
 (0)