BUG/MEDIUM: resolvers: fix name compression pointer validation in resolv_read_name()

The original DNS code would only use the 8 lower bits of the compression
offset. This was fixed in 2.0 with commit 2fa66c3b9 ("BUG/MEDIUM: dns:
overflowed dns name start position causing invalid dns error") but it was
not sufficient because the anti-loop check continues to use only 8 of the
14 bits, thus a crafted response where the 8 lower bits pass the check and
the 6 higher should fail it would be accepted. The impacts remains limited
thanks to the bounds check and the recursion limits, but such invalid
responses could still cost a lot to process. Let's compute the 14-bit
offset once for all and use it everywhere.

diff --git a/src/resolvers.c b/src/resolvers.c
index 518421ecaaa8c749144abdc5f39bca911471ddae..1d3d788a9f12120044aaefeb750cf0517117562b 100644 (file)
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -624,17 +624,21 @@ int resolv_read_name(unsigned char *buffer, unsigned char *bufend,
 
                /* Name compression is in use */
                if ((*reader & 0xc0) == 0xc0) {
+                       uint16_t ptr_offset;
+
                        if (reader + 1 >= bufend)
                                goto err;
 
+                       ptr_offset = (*reader & 0x3f) * 256 + reader[1];
+
                        /* Must point BEFORE current position */
-                       if ((buffer + reader[1]) > reader)
+                       if ((buffer + ptr_offset) >= reader)
                                goto err;
 
                        if (depth++ > 100)
                                goto err;
 
-                       n = resolv_read_name(buffer, bufend, buffer + (*reader & 0x3f)*256 + reader[1],
+                       n = resolv_read_name(buffer, bufend, buffer + ptr_offset,
                                             dest, dest_len - nb_bytes, offset, depth);
                        if (n == 0)
                                goto err;