christian.heimes <python-checkins@python.org> wrote:
Make sure that *really* no more than sizeof(ifr.ifr_name) chars are strcpy-ed to ifr.ifr_name and that the string is *always* NUL terminated. New code shouldn't use strcpy(), too. CID 719692
files: Modules/socketmodule.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -1674,7 +1674,8 @@ if (len == 0) { ifr.ifr_ifindex = 0; } else if (len < sizeof(ifr.ifr_name)) { - strcpy(ifr.ifr_name, PyBytes_AS_STRING(interfaceName)); + strncpy(ifr.ifr_name, PyBytes_AS_STRING(interfaceName), sizeof(ifr.ifr_name)); + ifr.ifr_name[(sizeof(ifr.ifr_name))-1] = '\0'; if (ioctl(s->sock_fd, SIOCGIFINDEX, &ifr) < 0) { s->errorhandler(); Py_DECREF(interfaceName);
I've trouble finding the overrun in the existing code. Previously: We have: 1) len = PyBytes_GET_SIZE(interfaceName); (without NUL terminator) At the point of strcpy() we have: 2) len < sizeof(ifr.ifr_name) PyBytes_AS_STRING(interfaceName) always adds a NUL terminator, so: 3) len+1 <= sizeof(ifr.ifr_name) 4) strcpy(ifr.ifr_name, PyBytes_AS_STRING(interfaceName)) will not overrun ifr.ifr_name and ifr.ifr_name is always NUL terminated. So IMO the strcpy() was safe and the report is a false positive. Stefan Krah