[Dnsmasq-discuss] possible overflow while constructing SRV record name

[Simon Kelley]

On 7/11/25 13:31, Mikhail Dmitrichenko wrote:
> Hello!
> 
> First of all, I want to clarify that I'm new to dnsmasq, so please 
> excuse me if my message seems silly.
> 
> My question is about constructing srv record name in function src/ 
> option.c/read_opts, v2.91. Specifically, this part:
> 
>    if (daemon->domain_suffix)
>      {
>         /* add domain for any srv record without one. */
>        struct mx_srv_record *srv;
> 
>        for (srv = daemon->mxnames; srv; srv = srv->next)
>      if (srv->issrv &&
>        strchr(srv->name, '.') &&
>        strchr(srv->name, '.') == strrchr(srv->name, '.'))
>       {
>        strcpy(buff, srv->name);
>        strcat(buff, ".");
>        strcat(buff, daemon->domain_suffix);
>        free(srv->name);
>        srv->name = opt_string_alloc(buff);
>       }
>      }
> 
> Here, `buff` is a buffer of length ((MAXDNAME * 2) + 1) bytes. If I 
> understand it correctly, after lines:
> 
>     strcpy(buff, srv->name);
>     strcat(buff, ".");
> 
> maximum len of `buff` will be (MAXDNAME + 1) bytes. My concern is about 
> next line:
> 
>     strcat(buff, daemon->domain_suffix);
> 
> As I see it, there is a possible scenario, where domain name in 
> `resolv.conf` contains non-ASCII symbols. If IDN library is present, 
> this name will be encoded and set as `daemon->domain_suffix` value. I 
> noticed that in function src/util.c/canonicalise, there is a call to 
> `check_name`, which checks len of given non-encoded input string. If it 
> exceeds `MAXDNAME`, NULL will be returned, so `daemon->domain_suffix` 
> will be NULL.
> 
> But what if encoded string exceeds `MAXDNAME`? I haven't found any check 
> for that. If this happens, it could cause overflow in the line
> 
>     strcat(buff, daemon->domain_suffix);
> 
> because `buff` may already contain (`MAXDNAME` + 1) bytes, leaving only 
> `MAXDNAME` bytes free. Do you think it will be useful to add a check of 
> encoded domain name length to prevent such an overflow? Any response 
> would be appreciated.
> 
> I'm aware that such long domain names are unusual and described scenario 
> is unlikely in real life, but I still wanted to highlight it.
> 
> Thank you in advance for your time and expertise!


First, the code that appends the domain to SRV records. This is a good 
point, but note that the both the domain and the name of SRV records 
come from the configuration of dnsmasq, they are not untrusted data from 
the internet. The anyone who can manipulate them has root on the machine 
running dnsmasq, so this it not a security problem. The worst case 
scenario is that a somewhat bizarre configuration causes dnsmasq to crah 
on startup. The fix is to check for overflow, and exit with an suitable 
error message, so it's easy to find and correct.


Aside: the buffer length is (MAXDNAME*2)+1 because dnsmasq uses an 
escape method to represent a few special characters in domain names. For 
instance '.' could appear in a valid label, but dnsmasq uses '.' in the 
obvious way as a label delimited. '.' in a label is therefore 
represented as the escape byte followed by a second byte. In theory, and 
name could require an escape character for each character, and a domain 
name is limited to MAXDNAME characters, hence the MAXDNAME * 2. The 
added one is for the zero terminator.

Second, the problem with returned encoded names from libidn - the 
documentation for idn2 states that domain names are limited to 255 
characters, which is less that MAXDNAME - I'm not sure why. libida is 
even more restrictive and 63 characters. I don't think that this a
therefore  a problem.

I've pushed a commit to make the check on the first issue.

Cheers,

Simon.

> 
> _______________________________________________
> Dnsmasq-discuss mailing list
> Dnsmasq-discuss at lists.thekelleys.org.uk
> https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss