[PATCHES] MS-PAR rpcclient

[Jeremy Allison]

On Wed, Nov 30, 2016 at 07:39:08AM +0100, Andreas Schneider wrote:
> >         for (i = 0; i < r->cfheader.cFolders; i++) {
> >                 count += r->cffolders[i].cCFData;
> >         }
> > 
> > Same there.
> > 
> > Pure paranoia of course, but these days I find
> > paranoia is usually justified :-).
> > 
> > What do you think ? If you agree I can code it
> > up for you to review.
> 
> I'm fine with adding it. It is also easier to spot if something really goes 
> wrong.

Here are the patches. Please review.

Thanks,

	Jeremy.
-------------- next part --------------
From a470dc3a1cf6e2849b139ed84fee88f069f1e140 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 30 Nov 2016 09:19:43 -0800
Subject: [PATCH 1/2] librpc: cab: Integer wrap protection for
 ndr_count_cfdata().

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 librpc/ndr/ndr_cab.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/librpc/ndr/ndr_cab.c b/librpc/ndr/ndr_cab.c
index 0d2b36b..d5ab87f 100644
--- a/librpc/ndr/ndr_cab.c
+++ b/librpc/ndr/ndr_cab.c
@@ -57,6 +57,10 @@ uint32_t ndr_count_cfdata(const struct cab_file *r)
 	uint32_t count = 0, i;
 
 	for (i = 0; i < r->cfheader.cFolders; i++) {
+		if (count + r->cffolders[i].cCFData < count) {
+			/* Integer wrap. */
+			return 0;
+		}
 		count += r->cffolders[i].cCFData;
 	}
 
-- 
2.8.0.rc3.226.g39d4020


From 7b8cf9c6815c903611635522343ef07db45e94b3 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 30 Nov 2016 09:23:52 -0800
Subject: [PATCH 2/2] librpc: cab: Fix ndr_size_cab_file() to detect integer
 wrap.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 librpc/ndr/ndr_cab.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/librpc/ndr/ndr_cab.c b/librpc/ndr/ndr_cab.c
index d5ab87f..eb28648 100644
--- a/librpc/ndr/ndr_cab.c
+++ b/librpc/ndr/ndr_cab.c
@@ -116,7 +116,7 @@ uint32_t ndr_cab_generate_checksum(const struct CFDATA *r)
 					csumPartial);
 }
 
-static uint32_t ndr_size_cab_file(const struct cab_file *r)
+static bool ndr_size_cab_file(const struct cab_file *r, uint32_t *psize)
 {
 	uint32_t size = 0;
 	int i;
@@ -126,20 +126,34 @@ static uint32_t ndr_size_cab_file(const struct cab_file *r)
 
 	/* folder */
 	for (i = 0; i < r->cfheader.cFolders; i++) {
+		if (size + 8 < size) {
+			/* Integer wrap. */
+			return false;
+		}
 		size += 8;
 	}
 
 	/* files */
 	for (i = 0; i < r->cfheader.cFiles; i++) {
-		size += ndr_size_CFFILE(&r->cffiles[i], 0);
+		uint32_t cfsize = ndr_size_CFFILE(&r->cffiles[i], 0);
+		if (size + cfsize < size) {
+			/* Integer wrap. */
+			return false;
+		}
+		size += cfsize;
 	}
 
 	/* data */
 	for (i = 0; i < ndr_count_cfdata(r); i++) {
+		if (size + r->cfdata[i].cbData < size) {
+			/* Integer wrap. */
+			return false;
+		}
 		size += 8 + r->cfdata[i].cbData;
 	}
 
-	return size;
+	*psize = size;
+	return true;
 }
 
 enum cf_compress_type ndr_cab_get_compression(const struct cab_file *r)
@@ -156,6 +170,7 @@ _PUBLIC_ enum ndr_err_code ndr_push_cab_file(struct ndr_push *ndr, int ndr_flags
 	uint32_t cntr_cffolders_0;
 	uint32_t cntr_cffiles_0;
 	uint32_t cntr_cfdata_0;
+	uint32_t cab_size = 0;
 	{
 		uint32_t _flags_save_STRUCT = ndr->flags;
 		ndr_set_flags(&ndr->flags, LIBNDR_PRINT_ARRAY_HEX|LIBNDR_FLAG_LITTLE_ENDIAN|LIBNDR_FLAG_NOALIGN);
@@ -188,7 +203,10 @@ _PUBLIC_ enum ndr_err_code ndr_push_cab_file(struct ndr_push *ndr, int ndr_flags
 		ndr->flags = _flags_save_STRUCT;
 	}
 
-	SIVAL(ndr->data, 8, ndr_size_cab_file(r));
+	if (ndr_size_cab_file(r, &cab_size) == false) {
+		return NDR_ERR_VALIDATE;
+	}
+	SIVAL(ndr->data, 8, cab_size);
 
 	return NDR_ERR_SUCCESS;
 }
-- 
2.8.0.rc3.226.g39d4020