search

jbagg / QtZeroConf

Qt wrapper class for ZeroConf libraries across various platforms.
Other
69 stars 51 forks source link

Segmentation fault while reading TXT records #45

Open FMeinicke opened 3 years ago

FMeinicke commented 3 years ago

I recently experienced a weird crash when using QtZeroConf on Windows:
I had multiple servers running on my local machine that announced themselves using python's zeroconf implementation. In my C++ client application, I used the zeroconf browser to discover them. While the browser was discovering the servers (more specifically, during parsing of the TXT records of the entries) my application cashed.

I could trace the cause of the crash to the QZeroConfPrivate::resolverCallback function in bonjour.cpp.
While iterating over the txtRecord array it happened that the length of the next record (recLen) was bigger than the total length of the TXT records (txtLen). This resulted in an overflow in this line https://github.com/jbagg/QtZeroConf/blob/df4a1581ab92de65cae69f522a13bb9a8b31e047/bonjour.cpp#L176 since txtLen is an unsigned int.

I could fix the problem by simply adding a check after the recLen was read (i.e. after this): https://github.com/jbagg/QtZeroConf/blob/df4a1581ab92de65cae69f522a13bb9a8b31e047/bonjour.cpp#L167

    if (recLen > txtLen)
    {
        break;
    }

I just wanted to get your opinion on this since I'm not really familiar with DNS records: Do you think this behaviour is just a bug in QtZeroConf or could it be that the python zeroconf implementation does something weird with the records it's publishing?
I just would've thought that it could never happen that a TXT record reports that its length (recLen) is 195 but the total length of all TXT records (txtLen) is just 41 or so.