unknown-payload: Use a new private payload type and make original type available
authorTobias Brunner <tobias@strongswan.org>
Fri, 15 May 2015 09:15:57 +0000 (11:15 +0200)
committerAndreas Steffen <andreas.steffen@strongswan.org>
Mon, 1 Jun 2015 07:42:11 +0000 (09:42 +0200)
This fixes a DoS and potential remote code execution vulnerability that was
caused because the original payload type that was returned previously was
used to cast such payload objects to payloads of the indicated type (e.g.
when logging notify payloads with a payload type for the wrong IKE version).

Fixes CVE-2015-3991.

src/libcharon/encoding/message.c
src/libcharon/encoding/payloads/payload.c
src/libcharon/encoding/payloads/payload.h
src/libcharon/encoding/payloads/unknown_payload.c
src/libcharon/encoding/payloads/unknown_payload.h
src/libcharon/sa/ikev2/task_manager_v2.c

index 1ee2cf8..478f531 100644 (file)
@@ -2513,7 +2513,7 @@ static status_t decrypt_payloads(private_message_t *this, keymat_t *keymat)
                        was_encrypted = "encrypted fragment payload";
                }
 
                        was_encrypted = "encrypted fragment payload";
                }
 
-               if (payload_is_known(type, this->major_version) && !was_encrypted &&
+               if (type != PL_UNKNOWN && !was_encrypted &&
                        !is_connectivity_check(this, payload) &&
                        this->exchange_type != AGGRESSIVE)
                {
                        !is_connectivity_check(this, payload) &&
                        this->exchange_type != AGGRESSIVE)
                {
index a1cd2f9..f7c2754 100644 (file)
@@ -97,6 +97,7 @@ ENUM_NEXT(payload_type_names, PLV1_NAT_D_DRAFT_00_03, PLV1_FRAGMENT, PLV2_FRAGME
 #endif /* ME */
 ENUM_NEXT(payload_type_names, PL_HEADER, PLV1_ENCRYPTED, PLV1_FRAGMENT,
        "HEADER",
 #endif /* ME */
 ENUM_NEXT(payload_type_names, PL_HEADER, PLV1_ENCRYPTED, PLV1_FRAGMENT,
        "HEADER",
+       "UNKNOWN",
        "PROPOSAL_SUBSTRUCTURE",
        "PROPOSAL_SUBSTRUCTURE_V1",
        "TRANSFORM_SUBSTRUCTURE",
        "PROPOSAL_SUBSTRUCTURE",
        "PROPOSAL_SUBSTRUCTURE_V1",
        "TRANSFORM_SUBSTRUCTURE",
@@ -167,6 +168,7 @@ ENUM_NEXT(payload_type_short_names, PLV1_NAT_D_DRAFT_00_03, PLV1_FRAGMENT, PLV2_
 #endif /* ME */
 ENUM_NEXT(payload_type_short_names, PL_HEADER, PLV1_ENCRYPTED, PLV1_FRAGMENT,
        "HDR",
 #endif /* ME */
 ENUM_NEXT(payload_type_short_names, PL_HEADER, PLV1_ENCRYPTED, PLV1_FRAGMENT,
        "HDR",
+       "UNKN",
        "PROP",
        "PROP",
        "TRANS",
        "PROP",
        "PROP",
        "TRANS",
index 920779b..7200389 100644 (file)
@@ -1,5 +1,5 @@
 /*
 /*
- * Copyright (C) 2007 Tobias Brunner
+ * Copyright (C) 2007-2015 Tobias Brunner
  * Copyright (C) 2005-2006 Martin Willi
  * Copyright (C) 2005 Jan Hutter
  * Hochschule fuer Technik Rapperswil
  * Copyright (C) 2005-2006 Martin Willi
  * Copyright (C) 2005 Jan Hutter
  * Hochschule fuer Technik Rapperswil
@@ -264,6 +264,11 @@ enum payload_type_t {
        PL_HEADER = 256,
 
        /**
        PL_HEADER = 256,
 
        /**
+        * Used to handle unknown or invalid payload types.
+        */
+       PL_UNKNOWN,
+
+       /**
         * PLV2_PROPOSAL_SUBSTRUCTURE, IKEv2 proposals in a SA payload.
         */
        PLV2_PROPOSAL_SUBSTRUCTURE,
         * PLV2_PROPOSAL_SUBSTRUCTURE, IKEv2 proposals in a SA payload.
         */
        PLV2_PROPOSAL_SUBSTRUCTURE,
index 45b91fd..c69254f 100644 (file)
@@ -1,4 +1,5 @@
 /*
 /*
+ * Copyright (C) 2015 Tobias Brunner
  * Copyright (C) 2005-2006 Martin Willi
  * Copyright (C) 2005 Jan Hutter
  * Hochschule fuer Technik Rapperswil
  * Copyright (C) 2005-2006 Martin Willi
  * Copyright (C) 2005 Jan Hutter
  * Hochschule fuer Technik Rapperswil
@@ -121,6 +122,12 @@ METHOD(payload_t, get_header_length, int,
 METHOD(payload_t, get_payload_type, payload_type_t,
        private_unknown_payload_t *this)
 {
 METHOD(payload_t, get_payload_type, payload_type_t,
        private_unknown_payload_t *this)
 {
+       return PL_UNKNOWN;
+}
+
+METHOD(unknown_payload_t, get_type, payload_type_t,
+       private_unknown_payload_t *this)
+{
        return this->type;
 }
 
        return this->type;
 }
 
@@ -181,6 +188,7 @@ unknown_payload_t *unknown_payload_create(payload_type_t type)
                                .destroy = _destroy,
                        },
                        .is_critical = _is_critical,
                                .destroy = _destroy,
                        },
                        .is_critical = _is_critical,
+                       .get_type = _get_type,
                        .get_data = _get_data,
                        .destroy = _destroy,
                },
                        .get_data = _get_data,
                        .destroy = _destroy,
                },
index 326b550..09341bc 100644 (file)
@@ -1,4 +1,5 @@
 /*
 /*
+ * Copyright (C) 2015 Tobias Brunner
  * Copyright (C) 2005-2006 Martin Willi
  * Copyright (C) 2005 Jan Hutter
  * Hochschule fuer Technik Rapperswil
  * Copyright (C) 2005-2006 Martin Willi
  * Copyright (C) 2005 Jan Hutter
  * Hochschule fuer Technik Rapperswil
@@ -42,6 +43,13 @@ struct unknown_payload_t {
        payload_t payload_interface;
 
        /**
        payload_t payload_interface;
 
        /**
+        * Get the original payload type as sent by the peer.
+        *
+        * @return                              type of the original payload
+        */
+       payload_type_t (*get_type) (unknown_payload_t *this);
+
+       /**
         * Get the raw data of this payload, without
         * the generic payload header.
         *
         * Get the raw data of this payload, without
         * the generic payload header.
         *
index 2981677..4676867 100644 (file)
@@ -1184,15 +1184,17 @@ static status_t parse_message(private_task_manager_t *this, message_t *msg)
                enumerator = msg->create_payload_enumerator(msg);
                while (enumerator->enumerate(enumerator, &payload))
                {
                enumerator = msg->create_payload_enumerator(msg);
                while (enumerator->enumerate(enumerator, &payload))
                {
-                       unknown = (unknown_payload_t*)payload;
-                       type = payload->get_type(payload);
-                       if (!payload_is_known(type, msg->get_major_version(msg)) &&
-                               unknown->is_critical(unknown))
+                       if (payload->get_type(payload) == PL_UNKNOWN)
                        {
                        {
-                               DBG1(DBG_ENC, "payload type %N is not supported, "
-                                        "but its critical!", payload_type_names, type);
-                               status = NOT_SUPPORTED;
-                               break;
+                               unknown = (unknown_payload_t*)payload;
+                               if (unknown->is_critical(unknown))
+                               {
+                                       type = unknown->get_type(unknown);
+                                       DBG1(DBG_ENC, "payload type %N is not supported, "
+                                                "but its critical!", payload_type_names, type);
+                                       status = NOT_SUPPORTED;
+                                       break;
+                               }
                        }
                }
                enumerator->destroy(enumerator);
                        }
                }
                enumerator->destroy(enumerator);