Skip to content
Permalink
Browse files

net: dns: Make dns_unpack_answer() to check non-compressed answers

Modify dns_unpack_answer() function to check if the answer is
compressed or not, and return correct values regardless.

Fixes #16594

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
  • Loading branch information...
jukkar committed Jun 25, 2019
1 parent f7697c4 commit ef1093755328e7ecab2965b273110d57b4d76931
Showing with 38 additions and 13 deletions.
  1. +38 −13 subsys/net/lib/dns/dns_pack.c
@@ -74,6 +74,8 @@ static inline void set_dns_msg_response(struct dns_msg_t *dns_msg, int type,

int dns_unpack_answer(struct dns_msg_t *dns_msg, int dname_ptr, u32_t *ttl)
{
int dname_len = DNS_COMMON_UINT_SIZE;
int i = 0;
u16_t buf_size;
u16_t pos;
u16_t len;
@@ -82,14 +84,33 @@ int dns_unpack_answer(struct dns_msg_t *dns_msg, int dname_ptr, u32_t *ttl)

answer = dns_msg->msg + dns_msg->answer_offset;

if (answer[0] < DNS_LABEL_MAX_SIZE) {
return -ENOMEM;
}
if (answer[i] == 0xc0 && answer[i + 1] == 0x0c) {

This comment has been minimized.

Copy link
@pfalcon

pfalcon Aug 19, 2019

Collaborator

This is surrealistic patch. So, this line. Mind that i is initialized to 0, and it can be reached only from start. So, it's equivalent to to if (answer[0] == 0xc0 && answer[1] == 0x0c).

Now let's look at following if, labeled by check_pointer. When we arrive as fallthrough, and answer[i] == 0xc0. And when we arrive via the label, the same is true! So, if (answer[i] < DNS_LABEL_MAX_SIZE) is always true.

But wait, WTF is if (answer[i] == 0xc0 && answer[i + 1] == 0x0c) in the first place? There's no such thing in DNS, what RFC1035 tells us is:

The pointer takes the form of a two octet sequence:

    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
    | 1  1|                OFFSET                   |
    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+

I.e., there can be an arbitrary 1st byte >=0xc0, and arbitrary 2nd byte.

So, sorry, but this patch is complete crap, and this processing needs to be rewritten from scratch.

check_pointer:
if (answer[i] < DNS_LABEL_MAX_SIZE) {
return -ENOMEM;
}

/* Recovery of the pointer value */
ptr = (((answer[0] & DNS_LABEL_MAX_SIZE) << 8) + answer[1]);
if (ptr != dname_ptr) {
return -ENOMEM;
/* Recovery of the pointer value */
ptr = ((answer[i] & DNS_LABEL_MAX_SIZE) << 8) + answer[i + 1];
if (ptr != dname_ptr) {
return -ENOMEM;
}
} else {
dname_len = answer[i++] + 1;
while (answer[i]) {
if (answer[i] == 0xc0 && answer[i + 1] == 0x0c) {
dname_len += DNS_COMMON_UINT_SIZE;
goto check_pointer;
}

if (answer[i] < DNS_LABEL_MAX_SIZE) {
dname_len += answer[i] + 1;
}

i++;
}

dname_len++;
}

/*
@@ -111,16 +132,20 @@ int dns_unpack_answer(struct dns_msg_t *dns_msg, int dname_ptr, u32_t *ttl)
/* Only DNS_CLASS_IN answers
* Here we use 2 as an offset because a ptr uses only 2 bytes.
*/
if (dns_answer_class(DNS_COMMON_UINT_SIZE, answer) != DNS_CLASS_IN) {
if (dns_answer_class(dname_len, answer) != DNS_CLASS_IN) {
return -EINVAL;
}

/* TTL value */
*ttl = dns_answer_ttl(DNS_COMMON_UINT_SIZE, answer);
pos = dns_msg->answer_offset + DNS_ANSWER_MIN_SIZE;
len = dns_answer_rdlength(DNS_COMMON_UINT_SIZE, answer);

switch (dns_answer_type(DNS_COMMON_UINT_SIZE, answer)) {
*ttl = dns_answer_ttl(dname_len, answer);
len = dns_answer_rdlength(dname_len, answer);
pos = dns_msg->answer_offset + dname_len +
DNS_COMMON_UINT_SIZE + /* class length */
DNS_COMMON_UINT_SIZE + /* type length */
DNS_TTL_LEN +
DNS_RDLENGTH_LEN;

switch (dns_answer_type(dname_len, answer)) {
case DNS_RR_TYPE_A:
case DNS_RR_TYPE_AAAA:
set_dns_msg_response(dns_msg, DNS_RESPONSE_IP, pos, len);

0 comments on commit ef10937

Please sign in to comment.
You can’t perform that action at this time.