From: mame@... Date: 2015-06-18T15:59:23+00:00 Subject: [ruby-core:69662] [Ruby trunk - Bug #11270] [Open] Coverity Scan warns out-of-bounds access in ext/socket Issue #11270 has been updated by Yusuke Endoh. Status changed from Feedback to Open Akira Tanaka wrote: > arg.alen is initialized as sizeof(union_sockaddr) and > modified by recvfrom() which is less than or equal to sizeof(union_sockaddr). > > rai is rb_addrinfo_t and rai->addr is union_sockaddr. > > So the memcpy() doesn't overflow. I think that Coverity Scan is warning against `sa`, not `rai`. `sa` is `&arg.buf.addr`, not `&arg.buf`. If it were `&arg.buf`, there is certainly no problem. Honestly I'm not sure the C language specification: is it guaranteed that a pointer to a field of a union and a pointer to the union itself? In short, `(void*)&arg.buf.addr == (void*)&arg.buf`? If it is guaranteed, there is no problem. But I couldn't find the guarantee from the specification. > > I don't think this inconsistency will cause actual harm, but it would be better to fix. > > Do you have an idea to fix it? > > I guess the inconsistency is caused by "struct sockaddr" is used as a type for generic socket addresses > but actually a fixed length buffer which may be not enough for some addresses. > It is a Unix tradition and too dificult to fix. I'm not familiar with socket apis. Do you mean that the apis are ill-designed so that we cannot use them in the strict C language? If so, I agree that it is difficult to fix. -- Yusuke Endoh ---------------------------------------- Bug #11270: Coverity Scan warns out-of-bounds access in ext/socket https://bugs.ruby-lang.org/issues/11270#change-53022 * Author: Yusuke Endoh * Status: Open * Priority: Normal * Assignee: * ruby -v: * Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN ---------------------------------------- Hello, Coverity Scan warns ext/socket/init.c and raddrinfo.c. `rsock_s_recvfrom` in ext/socket/init.c does: arg.alen = (socklen_t)sizeof(arg.buf); then calls `rsock_io_socket_addrinfo`: return rb_assoc_new(str, rsock_io_socket_addrinfo(sock, &arg.buf.addr, arg.alen)); `rsock_io_socket_addrinfo` indirectly calls `init_addrinfo` in ext/socket/raddrinfo.c. (`rsock_io_socket_addrinfo` -> `rsock_fd_socket_addrinfo` -> `rsock_addrinfo_new` -> `init_addrinfo`) `init_addrinfo` does: memcpy((void *)&rai->addr, (void *)sa, len); Note that `sa` is `&arg.buf.addr`, and `len` is `arg.alen`. `&arg.buf.addr` is a pointer to sockaddr, and `arg.len` is `sizeof(union_sockaddr)`, not `sizeof(sockaddr)`, which is indeed inconsistent. I don't think this inconsistency will cause actual harm, but it would be better to fix. -- Yusuke Endoh -- https://bugs.ruby-lang.org/