bacnet-stack icon indicating copy to clipboard operation
bacnet-stack copied to clipboard

Router application doesn't appear to correctly handle local broadcasts?

Open NiallBegley opened this issue 4 years ago • 0 comments

If I fire off what I understand to be a local broadcast - an IAm message with no destination information in the NPCI, for example - the router application interprets the lack of destination information as being destined for network 0, which causes it to then issue a NETWORK_MESSAGE_WHO_IS_ROUTER_TO_NETWORK message attempting to find network 0.

Here is the message I'm sending out:

image

And what I see is when this message gets passed to npdu_decode, it correctly checks to see if the control bit (5) is set or not and when it sees it is not, it zeros out our destination information:

/*Bit 5: Destination specifier where: */
        /* 0 = DNET, DLEN, DADR, and Hop Count absent */
        /* 1 = DNET, DLEN, and Hop Count present */
        /* DLEN = 0 denotes broadcast MAC DADR and DADR field is absent */
        /* DLEN > 0 specifies length of DADR field */
        if (npdu[1] & BIT5) {
            len += decode_unsigned16(&npdu[len], &dest_net);
            /* DLEN = 0 denotes broadcast MAC DADR and DADR field is absent */
            /* DLEN > 0 specifies length of DADR field */
            address_len = npdu[len++];
            if (dest) {
                dest->net = dest_net;
                dest->len = address_len;
            }
            if (address_len) {
                if (address_len > MAX_MAC_LEN) {
                    /* address is too large could be a malformed message */
                    return -1;
                }

                for (i = 0; i < address_len; i++) {
                    mac_octet = npdu[len++];
                    if (dest)
                        dest->adr[i] = mac_octet;
                }
            }
        }
        /* zero out the destination address */
        else if (dest) {
            dest->net = 0;
            dest->len = 0;
            for (i = 0; i < MAX_MAC_LEN; i++) {
                dest->adr[i] = 0;
            }
        }

Now when npdu_decode returns to process_msg, it attempts to find the appropriate destination:

    srcport = find_snet(msg->origin);
    destport = find_dnet(data->dest.net, NULL);
    assert(srcport);

    if (srcport && destport) {
        ....
    } else {
        /* request net search */
        return -1;
    }

This returns -1 since our destport cannot be found, which causes the NETWORK_MESSAGE_WHO_IS_ROUTER_TO_NETWORK message to be sent.

Am I correct in assuming this is a bug? Would an appropriate fix be to detect that the incoming message is a broadcast message with no destination information, assume that means the message is a local broadcast and discard the message entirely (since local broadcasts by definition should not extend beyond the router?)

NiallBegley avatar Feb 04 '21 18:02 NiallBegley