Re: [B.A.T.M.A.N.] [PATCH v3] [alfred] IPv4 multicast distribution support.

Sven Eckelmann
2017-01-19T11:51:02Z

[-- Attachment #1: Type: text/plain, Size: 1421 bytes --]

Just had a quick look at the ARP cache code while going to lunch (rest was not
checked):

> +int ipv4_to_mac(struct interface *interface,
> + const alfred_addr *addr, struct ether_addr *mac)
> +{
> + ipv4_arp_request(interface, addr, mac);
>
> if (!is_valid_ether_addr(mac->ether_addr_octet))
> return -EINVAL;
[...]
> +
> +int ipv4_arp_request(struct interface *interface, const alfred_addr *addr,
> + struct ether_addr *mac)
> +{
> + struct arpreq arpreq;
> + struct sockaddr_in *sin;
> +
> + memset(&arpreq, 0, sizeof(arpreq));
> +
> + sin = (struct sockaddr_in *)&arpreq.arp_pa;
> + sin->sin_family = AF_INET;
> + sin->sin_addr.s_addr = addr->ipv4.s_addr;
> +
> + strcpy(arpreq.arp_dev, interface->interface);
> + if (ioctl(interface->netsock, SIOCGARP, &arpreq) < 0)
> + return -1;
> +
> + if (arpreq.arp_flags & ATF_COM) {
> + memcpy(mac, arpreq.arp_ha.sa_data, sizeof(*mac));
> + } else {
> + perror("arp: incomplete");
> + return -1;
> + }
> +
> + return 0;
> +}

This looks like you are not always initializing/setting the data for mac. And
you are not checking the return value of ipv4_to_mac. So it is possible that
your are accessing "random"/"uninitialized" data in is_valid_ether_addr. It
can happen that your code accept bogus mac addresses (which are valid mac
addresses but are not actually the mac address of the remote system) - which
sounds wrong to me.