DNS Text Field Overflow

General
#1
  1. My goal is:
  • Preventing invalid mDNS text records.
  1. My actions are:
  • Calling mgos_dns_sd_add_service_instance with a set of key/value pairs that are more than 256 bytes.
  1. The result I see is:
  • A text record without a null terminator.
  1. My expectation & question is:
  • A valid DNS record with a null terminator.

The code in question is here: https://github.com/mongoose-os-libs/dns-sd/blob/master/src/mongoose/mgos_dns_sd.c#L425

For an example, let’s assume there’s already 250 bytes of data in the buffer. This means that p = buf + 250. On the next execution of the loop, we calculate p = 256 - (250) - 1 = 5. Since this is positive, we pass the check on line 427 and do not break out of the loop. We then call snprintf with the destination set to p + 1 == buf[251]. We can rely on snprintf to keep things within the 5 bytes we allow but this means we’re printing to buf[251], buf[252], buf[253], buf[254], and buf[255]. However, this means that the last byte of the buffer is not \0.

Since buf is then passed into mg_mk_str_n with the size as p - buf (in this example, it would be equal to 256), it should be OK. But it’s then passed into mg_strdup. You can see the source for that here: https://github.com/cesanta/mjs/blob/master/common/mg_str.c#L78. It is just a wrapper for mg_strdup_common (see https://github.com/cesanta/mjs/blob/master/common/mg_str.c#L61) with the nul_terminate parameter fixed to 0. This means it only allocates N bytes rather than N + 1.

Since it is possible to use the entire buffer, mg_strdup_nul should be called instead.