GNOME Bugzilla – Bug 589957
always overflow destination buffer
Last modified: 2018-08-17 13:36:17 UTC
Steps to reproduce: Compilation with check of oferflow string buffer Stack trace: This is excerpt from compilation log: 1017 ./gtk.override: In function '_wrap_gtk_message_dialog_format_secondary_text': 1018 ./gtk.override:6304: warning: format not a string literal and no format arguments 1019 ./gtk.override: In function '_wrap_gtk_message_dialog_format_secondary_markup': 1020 ./gtk.override:6327: warning: format not a string literal and no format arguments 1021 In file included from /usr/include/string.h:658, 1022 from /usr/include/python2.6/Python.h:38, 1023 from gdk.c:4: 1024 In function 'strncpy', 1025 inlined from '_wrap_gdk_event_tp_setattr' at gdkevent.override:357: 1026 /usr/include/bits/string3.h:122: error: call to __builtin___strncpy_chk will always overflow destination buffer Other information: We have a patch (by Sergey Vlasov <vsu at altlinux.ru>): diff --git a/pygtk/gtk/gdkevent.override b/pygtk/gtk/gdkevent.override index 3c0501d..7ae75de 100644 --- a/pygtk/gtk/gdkevent.override +++ b/pygtk/gtk/gdkevent.override @@ -352,12 +352,20 @@ _wrap_gdk_event_tp_setattr(PyObject *self, char *attr, PyObject *value) return 0; } else if (!strcmp(attr, "data")) { char *tmp; + Py_ssize_t len; STRING_CHECK(); - tmp = PyString_AsString(value); - strncpy((char *) &event->client.data, tmp, - sizeof(event->client.data)); + if (PyString_AsStringAndSize(value, &tmp, &len)) + return -1; + if (len < 0) + return -1; + if (len > sizeof(event->client.data)) + len = sizeof(event->client.data); + memcpy(&event->client.data, tmp, len); + memset((char *)&event->client.data + len, 0, + sizeof(event->client.data) - len); return 0; } + break; case GDK_VISIBILITY_NOTIFY: /*GdkEventVisibility visibility*/ if (!strcmp(attr, "state")) {
PS. This error throws on 64-bit architecture, not on i586.
Created attachment 140368 [details] [review] fix? I'm not sure I understand the problem. Seems to me the code is legit, given strncpy() description: > strncpy - Copy a length-limited, NUL-terminated string > > The result is not NUL-terminated if the source exceeds count bytes. > > In the case where the length of src is less than that of count, the > remainder of dest will be padded with NUL. The attached patch tries to clarify the code a little. But if the error is real, please explain it.
Created attachment 140371 [details] [review] fix? I'm not sure I understand the problem. Seems to me the code is legit, given strncpy() description: > strncpy - Copy a length-limited, NUL-terminated string > > The result is not NUL-terminated if the source exceeds count bytes. > > In the case where the length of src is less than that of count, the > remainder of dest will be padded with NUL. The attached patch tries to clarify the code a little. But if the error is real, please explain it.
Your patch fixes the main problem: the union size is different on different architectures, so sizeof(event->client.data) != sizeof(event->client.data.b). The patch proposed by Sergey Vlasov is just a more robust and, I'd say, paranoid way of doing things.
Paul, if you can read Russian, you can look here: http://lists.altlinux.org/pipermail/devel/2009-July/173499.html - for a detailed explanation made by Sergey himself.
pygtk is not under active development anymore and had its last code changes in 2013. Its codebase has been archived: https://gitlab.gnome.org/Archive/pygtk/commits/master PyGObject at https://gitlab.gnome.org/GNOME/pygobject is its successor. See https://pygobject.readthedocs.io/en/latest/guide/porting.html for porting info. Closing this report as WONTFIX as part of Bugzilla Housekeeping to reflect reality. Feel free to open a task in GNOME Gitlab if the issue described in this task still applies to a recent version of PyGObject. Thanks!