commit b003a0da7865caa25b5d1e70c79329b32409b02a (HEAD -> refs/heads/v4, refs/remotes/origin/v4)

Author: Amos Jeffries <yadij@users.noreply.github.com>

Date:   2021-09-24 21:53:11 +0000

    WCCP: Validate packets better (#899)

    Update WCCP to support exception based error handling for

    parsing and processing we are moving Squid to for protocol

    handling.

    Update the main WCCPv2 parsing checks to throw meaningful

    exceptions when detected.

diff --git a/src/wccp2.cc b/src/wccp2.cc

index ee592449c..6ef469e91 100644

--- a/src/wccp2.cc

+++ b/src/wccp2.cc

@@ -1108,6 +1108,59 @@ wccp2ConnectionClose(void)

  * Functions for handling the requests.

  */

+/// Checks that the given area section ends inside the given (whole) area.

+/// \param error the message to throw when the section does not fit

+static void

+CheckSectionLength(const void *sectionStart, const size_t sectionLength, const void *wholeStart, const size_t wholeSize, const char *error)

+{

+    assert(sectionStart);

+    assert(wholeStart);

+

+    const auto wholeEnd = static_cast<const char*>(wholeStart) + wholeSize;

+    assert(sectionStart >= wholeStart && "we never go backwards");

+    assert(sectionStart <= wholeEnd && "we never go beyond our whole (but zero-sized fields are OK)");

+    static_assert(sizeof(wccp2_i_see_you_t) <= PTRDIFF_MAX, "paranoid: no UB when subtracting in-whole pointers");

+    // subtraction safe due to the three assertions above

+    const auto remainderDiff = wholeEnd - static_cast<const char*>(sectionStart);

+

+    // casting safe due to the assertions above (and size_t definition)

+    assert(remainderDiff >= 0);

+    const auto remainderSize = static_cast<size_t>(remainderDiff);

+

+    if (sectionLength <= remainderSize)

+        return;

+

+    throw TextException(error, Here());

+}

+

+/// Checks that the area contains at least dataLength bytes after the header.

+/// The size of the field header itself is not included in dataLength.

+/// \returns the total field size -- the field header and field data combined

+template<class FieldHeader>

+static size_t

+CheckFieldDataLength(const FieldHeader *header, const size_t dataLength, const void *areaStart, const size_t areaSize, const char *error)

+{

+    assert(header);

+    const auto dataStart = reinterpret_cast<const char*>(header) + sizeof(header);

+    CheckSectionLength(dataStart, dataLength, areaStart, areaSize, error);

+    return sizeof(header) + dataLength; // no overflow after CheckSectionLength()

+}

+

+/// Positions the given field at a given start within a given packet area.

+/// The Field type determines the correct field size (used for bounds checking).

+/// \param field the field pointer the function should set

+/// \param areaStart the start of a packet (sub)structure containing the field

+/// \param areaSize the size of the packet (sub)structure starting at areaStart

+/// \param fieldStart the start of a field within the given area

+/// \param error the message to throw when the field does not fit the area

+template<class Field>

+static void

+SetField(Field *&field, const void *fieldStart, const void *areaStart, const size_t areaSize, const char *error)

+{

+    CheckSectionLength(fieldStart, sizeof(Field), areaStart, areaSize, error);

+    field = static_cast<Field*>(const_cast<void*>(fieldStart));

+}

+

 /*

  * Accept the UDP packet

  */

@@ -1124,8 +1177,6 @@ wccp2HandleUdp(int sock, void *)

     /* These structs form the parts of the packet */

-    struct wccp2_item_header_t *header = NULL;

-

     struct wccp2_security_none_t *security_info = NULL;

     struct wccp2_service_info_t *service_info = NULL;

@@ -1141,14 +1192,13 @@ wccp2HandleUdp(int sock, void *)

     struct wccp2_cache_identity_info_t *cache_identity = NULL;

     struct wccp2_capability_info_header_t *router_capability_header = NULL;

+    char *router_capability_data_start = nullptr;

     struct wccp2_capability_element_t *router_capability_element;

     struct sockaddr_in from;

     struct in_addr cache_address;

-    int len, found;

-    short int data_length, offset;

     uint32_t tmp;

     char *ptr;

     int num_caches;

@@ -1161,20 +1211,18 @@ wccp2HandleUdp(int sock, void *)

     Ip::Address from_tmp;

     from_tmp.setIPv4();

-    len = comm_udp_recvfrom(sock,

-                            &wccp2_i_see_you,

-                            WCCP_RESPONSE_SIZE,

-                            0,

-                            from_tmp);

+    const auto lenOrError = comm_udp_recvfrom(sock, &wccp2_i_see_you, WCCP_RESPONSE_SIZE, 0, from_tmp);

-    if (len < 0)

+    if (lenOrError < 0)

         return;

+    const auto len = static_cast<size_t>(lenOrError);

-    if (ntohs(wccp2_i_see_you.version) != WCCP2_VERSION)

-        return;

-

-    if (ntohl(wccp2_i_see_you.type) != WCCP2_I_SEE_YOU)

-        return;

+    try {

+        // TODO: Remove wccp2_i_see_you.data and use a buffer to read messages.

+        const auto message_header_size = sizeof(wccp2_i_see_you) - sizeof(wccp2_i_see_you.data);

+        Must2(len >= message_header_size, "incomplete WCCP message header");

+        Must2(ntohs(wccp2_i_see_you.version) == WCCP2_VERSION, "WCCP version unsupported");

+        Must2(ntohl(wccp2_i_see_you.type) == WCCP2_I_SEE_YOU, "WCCP packet type unsupported");

     /* FIXME INET6 : drop conversion boundary */

     from_tmp.getSockAddr(from);

@@ -1182,73 +1230,60 @@ wccp2HandleUdp(int sock, void *)

     debugs(80, 3, "Incoming WCCPv2 I_SEE_YOU length " << ntohs(wccp2_i_see_you.length) << ".");

     /* Record the total data length */

-    data_length = ntohs(wccp2_i_see_you.length);

+    const auto data_length = ntohs(wccp2_i_see_you.length);

+    Must2(data_length <= len - message_header_size,

+          "malformed packet claiming it's bigger than received data");

-    offset = 0;

-

-    if (data_length > len) {

-        debugs(80, DBG_IMPORTANT, "ERROR: Malformed WCCPv2 packet claiming it's bigger than received data");

-        return;

-    }

+    size_t offset = 0;

     /* Go through the data structure */

-    while (data_length > offset) {

+    while (offset + sizeof(struct wccp2_item_header_t) <= data_length) {

         char *data = wccp2_i_see_you.data;

-        header = (struct wccp2_item_header_t *) &data[offset];

+        const auto itemHeader = reinterpret_cast<const wccp2_item_header_t*>(&data[offset]);

+        const auto itemSize = CheckFieldDataLength(itemHeader, ntohs(itemHeader->length),

+                              data, data_length, "truncated record");

+        // XXX: Check "The specified length must be a multiple of 4 octets"

+        // requirement to avoid unaligned memory reads after the first item.

-        switch (ntohs(header->type)) {

+        switch (ntohs(itemHeader->type)) {

         case WCCP2_SECURITY_INFO:

-

-            if (security_info != NULL) {

-                debugs(80, DBG_IMPORTANT, "Duplicate security definition");

-                return;

-            }

-

-            security_info = (struct wccp2_security_none_t *) &wccp2_i_see_you.data[offset];

+            Must2(!security_info, "duplicate security definition");

+            SetField(security_info, itemHeader, itemHeader, itemSize,

+                     "security definition truncated");

             break;

         case WCCP2_SERVICE_INFO:

-

-            if (service_info != NULL) {

-                debugs(80, DBG_IMPORTANT, "Duplicate service_info definition");

-                return;

-            }

-

-            service_info = (struct wccp2_service_info_t *) &wccp2_i_see_you.data[offset];

+            Must2(!service_info, "duplicate service_info definition");

+            SetField(service_info, itemHeader, itemHeader, itemSize,

+                     "service_info definition truncated");

             break;

         case WCCP2_ROUTER_ID_INFO:

-

-            if (router_identity_info != NULL) {

-                debugs(80, DBG_IMPORTANT, "Duplicate router_identity_info definition");

-                return;

-            }

-

-            router_identity_info = (struct router_identity_info_t *) &wccp2_i_see_you.data[offset];

+            Must2(!router_identity_info, "duplicate router_identity_info definition");

+            SetField(router_identity_info, itemHeader, itemHeader, itemSize,

+                     "router_identity_info definition truncated");

             break;

         case WCCP2_RTR_VIEW_INFO:

-

-            if (router_view_header != NULL) {

-                debugs(80, DBG_IMPORTANT, "Duplicate router_view definition");

-                return;

-            }

-

-            router_view_header = (struct router_view_t *) &wccp2_i_see_you.data[offset];

+            Must2(!router_view_header, "duplicate router_view definition");

+            SetField(router_view_header, itemHeader, itemHeader, itemSize,

+                     "router_view definition truncated");

             break;

-        case WCCP2_CAPABILITY_INFO:

-

-            if (router_capability_header != NULL) {

-                debugs(80, DBG_IMPORTANT, "Duplicate router_capability definition");

-                return;

-            }

+        case WCCP2_CAPABILITY_INFO: {

+            Must2(!router_capability_header, "duplicate router_capability definition");

+            SetField(router_capability_header, itemHeader, itemHeader, itemSize,

+                     "router_capability definition truncated");

-            router_capability_header = (struct wccp2_capability_info_header_t *) &wccp2_i_see_you.data[offset];

+            CheckFieldDataLength(router_capability_header, ntohs(router_capability_header->capability_info_length),

+                                 itemHeader, itemSize, "capability info truncated");

+            router_capability_data_start = reinterpret_cast<char*>(router_capability_header) +

+                                           sizeof(*router_capability_header);

             break;

+        }

         /* Nothing to do for the types below */

@@ -1257,22 +1292,17 @@ wccp2HandleUdp(int sock, void *)

             break;

         default:

-            debugs(80, DBG_IMPORTANT, "Unknown record type in WCCPv2 Packet (" << ntohs(header->type) << ").");

+            debugs(80, DBG_IMPORTANT, "Unknown record type in WCCPv2 Packet (" << ntohs(itemHeader->type) << ").");

         }

-        offset += sizeof(struct wccp2_item_header_t);

-        offset += ntohs(header->length);

-

-        if (offset > data_length) {

-            debugs(80, DBG_IMPORTANT, "Error: WCCPv2 packet tried to tell us there is data beyond the end of the packet");

-            return;

-        }

+        offset += itemSize;

+        assert(offset <= data_length && "CheckFieldDataLength(itemHeader...) established that");

     }

-    if ((security_info == NULL) || (service_info == NULL) || (router_identity_info == NULL) || (router_view_header == NULL)) {

-        debugs(80, DBG_IMPORTANT, "Incomplete WCCPv2 Packet");

-        return;

-    }

+    Must2(security_info, "packet missing security definition");

+    Must2(service_info, "packet missing service_info definition");

+    Must2(router_identity_info, "packet missing router_identity_info definition");

+    Must2(router_view_header, "packet missing router_view definition");

     debugs(80, 5, "Complete packet received");

@@ -1308,10 +1338,7 @@ wccp2HandleUdp(int sock, void *)

             break;

     }

-    if (router_list_ptr->next == NULL) {

-        debugs(80, DBG_IMPORTANT, "WCCPv2 Packet received from unknown router");

-        return;

-    }

+    Must2(router_list_ptr->next, "packet received from unknown router");

     /* Set the router id */

     router_list_ptr->info->router_address = router_identity_info->router_id_element.router_address;

@@ -1331,11 +1358,20 @@ wccp2HandleUdp(int sock, void *)

         }

     } else {

-        char *end = ((char *) router_capability_header) + sizeof(*router_capability_header) + ntohs(router_capability_header->capability_info_length) - sizeof(struct wccp2_capability_info_header_t);

-

-        router_capability_element = (struct wccp2_capability_element_t *) (((char *) router_capability_header) + sizeof(*router_capability_header));

-

-        while ((char *) router_capability_element <= end) {

+        const auto router_capability_data_length = ntohs(router_capability_header->capability_info_length);

+        assert(router_capability_data_start);

+        const auto router_capability_data_end = router_capability_data_start +

+                                                router_capability_data_length;

+        for (auto router_capability_data_current = router_capability_data_start;

+                router_capability_data_current < router_capability_data_end;) {

+

+            SetField(router_capability_element, router_capability_data_current,

+                     router_capability_data_start, router_capability_data_length,

+                     "capability element header truncated");

+            const auto elementSize = CheckFieldDataLength(

+                                         router_capability_element, ntohs(router_capability_element->capability_length),

+                                         router_capability_data_start, router_capability_data_length,

+                                         "capability element truncated");

             switch (ntohs(router_capability_element->capability_type)) {

@@ -1377,7 +1413,7 @@ wccp2HandleUdp(int sock, void *)

                 debugs(80, DBG_IMPORTANT, "Unknown capability type in WCCPv2 Packet (" << ntohs(router_capability_element->capability_type) << ").");

             }

-            router_capability_element = (struct wccp2_capability_element_t *) (((char *) router_capability_element) + sizeof(struct wccp2_item_header_t) + ntohs(router_capability_element->capability_length));

+            router_capability_data_current += elementSize;

         }

     }

@@ -1396,23 +1432,34 @@ wccp2HandleUdp(int sock, void *)

     num_caches = 0;

     /* Check to see if we're the master cache and update the cache list */

-    found = 0;

+    bool found = false;

     service_list_ptr->lowest_ip = 1;

     cache_list_ptr = &router_list_ptr->cache_list_head;

     /* to find the list of caches, we start at the end of the router view header */

     ptr = (char *) (router_view_header) + sizeof(struct router_view_t);

+    const auto router_view_size = sizeof(struct router_view_t) +

+                                  ntohs(router_view_header->header.length);

     /* Then we read the number of routers */

-    memcpy(&tmp, ptr, sizeof(tmp));

+    const uint32_t *routerCountRaw = nullptr;

+    SetField(routerCountRaw, ptr, router_view_header, router_view_size,

+             "malformed packet (truncated router view info w/o number of routers)");

     /* skip the number plus all the ip's */

-

-    ptr += sizeof(tmp) + (ntohl(tmp) * sizeof(struct in_addr));

+    ptr += sizeof(*routerCountRaw);

+    const auto ipCount = ntohl(*routerCountRaw);

+    const auto ipsSize = ipCount * sizeof(struct in_addr); // we check for unsigned overflow below

+    Must2(ipsSize / sizeof(struct in_addr) != ipCount, "huge IP address count");

+    CheckSectionLength(ptr, ipsSize, router_view_header, router_view_size, "invalid IP address count");

+    ptr += ipsSize;

     /* Then read the number of caches */

-    memcpy(&tmp, ptr, sizeof(tmp));

+    const uint32_t *cacheCountRaw = nullptr;

+    SetField(cacheCountRaw, ptr, router_view_header, router_view_size,

+             "malformed packet (truncated router view info w/o cache count)");

+    memcpy(&tmp, cacheCountRaw, sizeof(tmp)); // TODO: Replace tmp with cacheCount

     ptr += sizeof(tmp);

     if (ntohl(tmp) != 0) {

@@ -1426,7 +1473,8 @@ wccp2HandleUdp(int sock, void *)

             case WCCP2_ASSIGNMENT_METHOD_HASH:

-                cache_identity = (struct wccp2_cache_identity_info_t *) ptr;

+                SetField(cache_identity, ptr, router_view_header, router_view_size,

+                         "malformed packet (truncated router view info cache w/o assignment hash)");

                 ptr += sizeof(struct wccp2_cache_identity_info_t);

@@ -1437,13 +1485,15 @@ wccp2HandleUdp(int sock, void *)

             case WCCP2_ASSIGNMENT_METHOD_MASK:

-                cache_mask_info = (struct cache_mask_info_t *) ptr;

+                SetField(cache_mask_info, ptr, router_view_header, router_view_size,

+                         "malformed packet (truncated router view info cache w/o assignment mask)");

                 /* The mask assignment has an undocumented variable length entry here */

                 if (ntohl(cache_mask_info->num1) == 3) {

-                    cache_mask_identity = (struct wccp2_cache_mask_identity_info_t *) ptr;

+                    SetField(cache_mask_identity, ptr, router_view_header, router_view_size,

+                             "malformed packet (truncated router view info cache w/o assignment mask identity)");

                     ptr += sizeof(struct wccp2_cache_mask_identity_info_t);

@@ -1474,10 +1524,7 @@ wccp2HandleUdp(int sock, void *)

             debugs (80, 5,  "checking cache list: (" << std::hex << cache_address.s_addr << ":" <<  router_list_ptr->local_ip.s_addr << ")");

             /* Check to see if it's the master, or us */

-

-            if (cache_address.s_addr == router_list_ptr->local_ip.s_addr) {

-                found = 1;

-            }

+            found = found || (cache_address.s_addr == router_list_ptr->local_ip.s_addr);

             if (cache_address.s_addr < router_list_ptr->local_ip.s_addr) {

                 service_list_ptr->lowest_ip = 0;

@@ -1494,7 +1541,7 @@ wccp2HandleUdp(int sock, void *)

         cache_list_ptr->next = NULL;

         service_list_ptr->lowest_ip = 1;

-        found = 1;

+        found = true;

         num_caches = 1;

     }

@@ -1502,7 +1549,7 @@ wccp2HandleUdp(int sock, void *)

     router_list_ptr->num_caches = htonl(num_caches);

-    if ((found == 1) && (service_list_ptr->lowest_ip == 1)) {

+    if (found && (service_list_ptr->lowest_ip == 1)) {

         if (ntohl(router_view_header->change_number) != router_list_ptr->member_change) {

             debugs(80, 4, "Change detected - queueing up new assignment");

             router_list_ptr->member_change = ntohl(router_view_header->change_number);

@@ -1515,6 +1562,10 @@ wccp2HandleUdp(int sock, void *)

         eventDelete(wccp2AssignBuckets, NULL);

         debugs(80, 5, "I am not the lowest ip cache - not assigning buckets");

     }

+

+    } catch (...) {

+        debugs(80, DBG_IMPORTANT, "ERROR: Ignoring WCCPv2 message: " << CurrentException);

+    }

 }

 static void