troglobit / mdnsd

Jeremie Miller's original mdnsd
BSD 3-Clause "New" or "Revised" License
56 stars 35 forks source link

Publishing colliding records #21

Closed bong1991 closed 4 years ago

bong1991 commented 4 years ago

Hey!

I found an issue, when _namehash % SPRIME leads to the same index and the name of records stored at that index differ.

mdnsd_find tries to find matching records by only comparing the type. This function is used by the function record in conf.c. When mdnsd_find finds a record now with matching type (E.g. Type SRV), then it enters the while loop (~conf.c:129) and it will never create a new record using mdnsd_shared or unique, although, we do not want to update an existing entry but rather create a new one.

In my case, I added three services, of which 2 of them had the same index and only one of them was correctly announced. Upon dns-sd query, it was shown but mdnsd was not able to answer when some instance requested the TXT record.

Adding strcmp(data->name, name) == 0 to mdnsd_find as a requirement, fixes the issue. mdnsd_shared takes care of placing it into the linked list at index _namehash(host) % SPRIME.

mdns_record_t *mdnsd_find(mdns_daemon_t *d, const char *name, unsigned short type)
{
    mdns_record_t *r;

    r = mdnsd_get_published(d, name);
    while (r) {
        const mdns_answer_t *data;

        data = mdnsd_record_data(r);
        // Search for a record with the same type and name. Records with different names might be in the same linked list
        // when the hash functions % SPRIME assigns them the same index (hash collision)
        if (data->type == type && strcmp(data->name, name) == 0)
            return r;

        r = mdnsd_record_next(r);
    }

    return NULL;
}
troglobit commented 4 years ago

Cool, nice find! :-) Would you like to submit a pull request, or wait for me to circle back to mdnsd and integrate the change? (I'm a bit busy so it might take a couple of weeks)

bong1991 commented 4 years ago

I will create a pull request :)

troglobit commented 4 years ago

Merged in PR #22, thanks again! :-)