From: akr@...
Date: 2015-06-19T01:36:47+00:00
Subject: [ruby-core:69672] [Ruby trunk - Bug #11270] Coverity Scan warns out-of-bounds access in ext/socket

Issue #11270 has been updated by Akira Tanaka.


Yusuke Endoh wrote:

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

Yes.

There is a description in the committee draft of C99.

```
       [#13] The size of a  union  is  sufficient  to  contain  the
       largest  of  its  members.   The value of at most one of the
       members can be stored in a union  object  at  any  time.   A
       pointer  to  a  union  object, suitably converted, points to
       each of its members (or if a member is a bit-field, then  to
       the unit in which it resides), and vice versa.
```

This section is quoted to Wikipedia (from C90).
https://en.wikipedia.org/wiki/Union_type

JIS X 3010:2003 section 6.7.2.1 has same description in Japanese.

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

I don't say "cannot" here.


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

* 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 <mame@ruby-lang.org>



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