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>

2 files changed, 49 insertions, 18 deletions

diff --git a/librpc/rpc/dcerpc_util.c b/librpc/rpc/dcerpc_util.c
index 696bcdae56e..8f0ebd01cfe 100644
--- 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 b03b9ffc814..f6434e7613d 100644
--- 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);