[PATCHES] MS-PAR rpcclient
Jeremy Allison
jra at samba.org
Wed Nov 30 18:22:30 UTC 2016
On Wed, Nov 30, 2016 at 12:33:25PM -0500, jim wrote:
> The integer wrap check is different from the increment - it is
> missing '8 +'.
Good catch - thanks ! Updated patch attached.
Jeremy.
> On 11/30/2016 12:29 PM, Jeremy Allison wrote:
> > /* 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;
>
>
-------------- next part --------------
From 17a38fd4b4c24966e520c85d71a8aee7e9f2666d 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 433e8e46ed2674e02d16a88bc47feae67bd51697 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 | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/librpc/ndr/ndr_cab.c b/librpc/ndr/ndr_cab.c
index d5ab87f..cc246a4 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,38 @@ 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 + 8 < size) {
+ /* Integer wrap. */
+ return false;
+ }
+ 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 +174,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 +207,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
More information about the samba-technical
mailing list