svn commit: r309933 - head/usr.sbin/syslogd

[Conrad Meyer]

On Mon, Dec 12, 2016 at 11:33 AM, Hiroki Sato <hrs at freebsd.org> wrote:
> Author: hrs
> Date: Mon Dec 12 19:33:40 2016
> New Revision: 309933
> URL: https://svnweb.freebsd.org/changeset/base/309933
>
> Log:
>   - Refactor listening socket list.  All of the listening sockets are
>     now maintained in a single linked-list in a transport-independent manner.
>   - Use queue.h for linked-list structure.
>   - Use linked-list for AllowedPeers.
>   - Use getaddrinfo(8) even for Unix Domain sockets.
>   - Use macros to type-casting from/to struct sockaddr{,_in,_in6}.
>   - Define fu_* macro for union f_un to shorten the member names.
>   - Remove an extra #include <sys/type.h>.
>   - Add "static" to non-exported symbols.
>   - !INET support is still incomplete but will be fixed later.
>
>   There is no functional change except for some minor debug messages.

Hello Hiroki,

This refactor introduced a bug in the IPv6 address comparison/rejection logic.

> Modified: head/usr.sbin/syslogd/syslogd.c
> ==============================================================================
> --- head/usr.sbin/syslogd/syslogd.c     Mon Dec 12 19:26:55 2016        (r309932)
> +++ head/usr.sbin/syslogd/syslogd.c     Mon Dec 12 19:33:40 2016        (r309933)
> ...
>      reject = 0;
> - for (j = 0; j < 16; j += 4) {
> - if ((*(u_int32_t *)&sin6->sin6_addr.s6_addr[j] & *(u_int32_t *)&m6p->sin6_addr.s6_addr[j])
> -    != *(u_int32_t *)&a6p->sin6_addr.s6_addr[j]) {
> - ++reject;
> - break;
> - }
> + if (IN6_ARE_MASKED_ADDR_EQUAL(&sin6->sin6_addr,
> +    &a6p->sin6_addr, &m6p->sin6_addr) != 0) {
> + ++reject;
> + break;
>      }
>      if (reject) {
>       dprintf("rejected in rule %d due to IP mismatch.\n", i);

The new check isn't a loop, so the 'break' breaks out of the outer
loop, which is unintentional.  I think we should just remove 'break'.

This was found by Coverity CID 1366941.

Best,
Conrad