mjansson / mdns

Public domain mDNS/DNS-SD library in C
The Unlicense
437 stars 117 forks source link

mdns_string_equal needs to be modified #57

Closed heshanu3d closed 2 years ago

heshanu3d commented 2 years ago

function mdns_string_equal should be modified like this int mdns_string_equal(const void buffer_lhs, size_t size_lhs, size_t ofs_lhs, const void buffer_rhs, size_t size_rhs, size_t ofs_rhs) { …… size_t compare_res = 1; do { …… if (compare_res && lhs_substr.length != rhs_substr.length) compare_res = 0; // return 0; if (compare_res && strncasecmp((const char ) MDNS_POINTER_OFFSET_CONST(buffer_rhs, rhs_substr.offset), (const char ) MDNS_POINTER_OFFSET_CONST(buffer_lhs, lhs_substr.offset), rhs_substr.length)) compare_res = 0; // return 0; …… } while (lhs_substr.length);

……

return compare_res;

}

mjansson commented 2 years ago

Why would it need to be modified like that? The offsets don't need values if the equality test fails.

heshanu3d commented 2 years ago

send a packet contains two queries like query0 "_ipps._tcp.local" and query1 "_ipp._tcp.local", and receive mdns packet contains two answers like answer0 "_ipps._tcp.local" and answer1 "_ipp.ref". if what we needed is "_ipp._tcp.local", the codes will go wrong.

    for (i = 0; i < answer_rrs; ++i) {
        size_t ofs = MDNS_POINTER_DIFF(data, buffer);
        size_t verify_ofs = 12;
        // Verify it's an answer to our question, _services._dns-sd._udp.local.
        size_t name_offset = ofs;
        int is_answer = mdns_string_equal(buffer, data_size, &ofs, mdns_services_query,
                                          sizeof(mdns_services_query), &verify_ofs);
        size_t name_length = ofs - name_offset;

when the code tries to parse the answer0, mdns_string_equal returns 0 in

if (lhs_substr.length != rhs_substr.length)
    return 0;

then ofs variable will not be updated after mdns_string_equal(buffer, data_size, &ofs, mdns_services_query, sizeof(mdns_services_query), &verify_ofs);

and answer1 will not be parsed correctly

mjansson commented 2 years ago

It is by design, but there is indeed a string skip missing in that answer processing loop - should be fixed now (including some variable naming cleanup)