CVE-2015-5370: librpc/rpc: simplify and harden dcerpc_pull_auth_trailer()

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11344

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Günther Deschner <gd@samba.org>

diff --git a/librpc/rpc/dcerpc_util.c b/librpc/rpc/dcerpc_util.c
index 696bcdae56ec39740eff59bb36d0ec81d3255c9d..8f0ebd01cfe97ce983d5b39d293e37d95eee8c0f 100644 (file)
--- a/librpc/rpc/dcerpc_util.c
+++ b/librpc/rpc/dcerpc_util.c
@@ -83,31 +83,44 @@ uint8_t dcerpc_get_endian_flag(DATA_BLOB *blob)
 *
 * @return              - A NTSTATUS error code.
 */
-NTSTATUS dcerpc_pull_auth_trailer(struct ncacn_packet *pkt,
+NTSTATUS dcerpc_pull_auth_trailer(const struct ncacn_packet *pkt,
                                  TALLOC_CTX *mem_ctx,
-                                 DATA_BLOB *pkt_trailer,
+                                 const DATA_BLOB *pkt_trailer,
                                  struct dcerpc_auth *auth,
-                                 uint32_t *auth_length,
+                                 uint32_t *_auth_length,
                                  bool auth_data_only)
 {
        struct ndr_pull *ndr;
        enum ndr_err_code ndr_err;
-       uint32_t data_and_pad;
+       uint16_t data_and_pad;
+       uint16_t auth_length;
+       uint32_t tmp_length;
 
-       data_and_pad = pkt_trailer->length
-                       - (DCERPC_AUTH_TRAILER_LENGTH + pkt->auth_length);
+       ZERO_STRUCTP(auth);
+       if (_auth_length != NULL) {
+               *_auth_length = 0;
+       }
 
-       /* paranoia check for pad size. This would be caught anyway by
-          the ndr_pull_advance() a few lines down, but it scared
-          Jeremy enough for him to call me, so we might as well check
-          it now, just to prevent someone posting a bogus YouTube
-          video in the future.
-       */
-       if (data_and_pad > pkt_trailer->length) {
-               return NT_STATUS_INFO_LENGTH_MISMATCH;
+       /* Paranoia checks for auth_length. The caller should check this... */
+       if (pkt->auth_length > pkt->frag_length) {
+               return NT_STATUS_INTERNAL_ERROR;
+       }
+       tmp_length = DCERPC_NCACN_PAYLOAD_OFFSET;
+       tmp_length += DCERPC_AUTH_TRAILER_LENGTH;
+       tmp_length += pkt->auth_length;
+       if (tmp_length > pkt->frag_length) {
+               return NT_STATUS_INTERNAL_ERROR;
+       }
+       if (pkt_trailer->length > UINT16_MAX) {
+               return NT_STATUS_INTERNAL_ERROR;
        }
 
-       *auth_length = pkt_trailer->length - data_and_pad;
+       auth_length = DCERPC_AUTH_TRAILER_LENGTH + pkt->auth_length;
+       if (pkt_trailer->length < auth_length) {
+               return NT_STATUS_RPC_PROTOCOL_ERROR;
+       }
+
+       data_and_pad = pkt_trailer->length - auth_length;
 
        ndr = ndr_pull_init_blob(pkt_trailer, mem_ctx);
        if (!ndr) {
@@ -127,14 +140,28 @@ NTSTATUS dcerpc_pull_auth_trailer(struct ncacn_packet *pkt,
        ndr_err = ndr_pull_dcerpc_auth(ndr, NDR_SCALARS|NDR_BUFFERS, auth);
        if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
                talloc_free(ndr);
+               ZERO_STRUCTP(auth);
                return ndr_map_error2ntstatus(ndr_err);
        }
 
+       if (data_and_pad < auth->auth_pad_length) {
+               DEBUG(1, (__location__ ": ERROR: pad length mismatch. "
+                         "Calculated %u  got %u\n",
+                         (unsigned)data_and_pad,
+                         (unsigned)auth->auth_pad_length));
+               talloc_free(ndr);
+               ZERO_STRUCTP(auth);
+               return NT_STATUS_RPC_PROTOCOL_ERROR;
+       }
+
        if (auth_data_only && data_and_pad != auth->auth_pad_length) {
-               DEBUG(1, (__location__ ": WARNING: pad length mismatch. "
+               DEBUG(1, (__location__ ": ERROR: pad length mismatch. "
                          "Calculated %u  got %u\n",
                          (unsigned)data_and_pad,
                          (unsigned)auth->auth_pad_length));
+               talloc_free(ndr);
+               ZERO_STRUCTP(auth);
+               return NT_STATUS_RPC_PROTOCOL_ERROR;
        }
 
        DEBUG(6,(__location__ ": auth_pad_length %u\n",
@@ -143,6 +170,10 @@ NTSTATUS dcerpc_pull_auth_trailer(struct ncacn_packet *pkt,
        talloc_steal(mem_ctx, auth->credentials.data);
        talloc_free(ndr);
 
+       if (_auth_length != NULL) {
+               *_auth_length = auth_length;
+       }
+
        return NT_STATUS_OK;
 }
 

diff --git a/librpc/rpc/rpc_common.h b/librpc/rpc/rpc_common.h
index b03b9ffc81432c4e7f402cce166a59841cc0f616..f6434e7613d6a037e544bea462f154ad2fb88152 100644 (file)
--- a/librpc/rpc/rpc_common.h
+++ b/librpc/rpc/rpc_common.h
@@ -187,9 +187,9 @@ const char *dcerpc_default_transport_endpoint(TALLOC_CTX *mem_ctx,
 *
 * @return              - A NTSTATUS error code.
 */
-NTSTATUS dcerpc_pull_auth_trailer(struct ncacn_packet *pkt,
+NTSTATUS dcerpc_pull_auth_trailer(const struct ncacn_packet *pkt,
                                  TALLOC_CTX *mem_ctx,
-                                 DATA_BLOB *pkt_trailer,
+                                 const DATA_BLOB *pkt_trailer,
                                  struct dcerpc_auth *auth,
                                  uint32_t *auth_length,
                                  bool auth_data_only);