[ruby-core:69647] [Ruby trunk - Bug #11270] [Feedback] Coverity Scan warns out-of-bounds access in ext/socket

Issue #11270 has been updated by Akira Tanaka.

Status changed from Open to Feedback

I'm not sure the problem.

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 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.




----------------------------------------
Bug #11270: Coverity Scan warns out-of-bounds access in ext/socket
https://bugs.ruby-lang.org/issues/11270#change-53001

* Author: Yusuke Endoh
* Status: Feedback
* 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 <[e-mail]>



-- 
https://bugs.ruby-lang.org/