Attachment 18256 Details for Bug 15579 – backport of master patch to 4.20

[patch] backport of master patch to 4.20

array-check-4.20-backport-option1.patch (text/plain), 6.22 KB, created by Noel Power on 2024-02-20 12:02:20 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Noel Power

Created: 2024-02-20 12:02:20 UTC

Size: 6.22 KB

patch

obsolete

>From 0225491b541aa54a52e9253b2be551a67b0d77d9 Mon Sep 17 00:00:00 2001
>From: Noel Power <noel.power@suse.com>
>Date: Wed, 14 Feb 2024 12:01:28 +0000
>Subject: [PATCH 1/3] idl: Add constant for max rows buffer size
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15579
>Signed-off-by: Noel Power <noel.power@suse.com>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(cherry picked from commit 01e901ef869a1a87fba0e67bce311dbeb199b717)
>---
> librpc/idl/wsp_data.idl | 5 +++++
> 1 file changed, 5 insertions(+)
>
>diff --git a/librpc/idl/wsp_data.idl b/librpc/idl/wsp_data.idl
>index 2a94355b0b0..fde754aef81 100644
>--- a/librpc/idl/wsp_data.idl
>+++ b/librpc/idl/wsp_data.idl
>@@ -11,6 +11,11 @@ interface constants
> 	 * for details of this and other language id(s)
> 	 */
> 	const uint32_t WSP_DEFAULT_LCID = 0x00000409;
>+	/*
>+	 * Max size of rows buffer in getrowsout response
>+	 * see MS-WSP 2.2.3.11
>+	 */
>+	const uint32_t MAX_ROW_BUFF_SIZE = 0x0004000;
> 
> 	/* values for guidPropertySet */
> 	const char* DBPROPSET_FSCIFRMWRK_EXT = "A9BD1526-6A80-11D0-8C9D-0020AF1D740E";
>-- 
>2.35.3
>
>
>From 6ccea75f5e4859c67f4f09940f8fdea4ae11dcb7 Mon Sep 17 00:00:00 2001
>From: Noel Power <noel.power@suse.com>
>Date: Wed, 14 Feb 2024 11:19:39 +0000
>Subject: [PATCH 2/3] s3/rpc_client: Ensure max possible row buffer size is not
> exceeded
>
>The max buf size of rows buffer should not exceed 0x00004000.
>Ensuring this value is within limits means we can safely use
>uint32_t offsets.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15579
>Signed-off-by: Noel Power <noel.power@suse.com>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(cherry picked from commit f487211706a74d516bf447ed393222b4c0dce7b0)
>---
> source3/rpc_client/wsp_cli.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
>diff --git a/source3/rpc_client/wsp_cli.c b/source3/rpc_client/wsp_cli.c
>index b21f55c86f0..1ce28e61807 100644
>--- a/source3/rpc_client/wsp_cli.c
>+++ b/source3/rpc_client/wsp_cli.c
>@@ -1311,6 +1311,19 @@ enum ndr_err_code extract_rowsarray(
> {
> 	uint32_t i;
> 	enum ndr_err_code err  = NDR_ERR_SUCCESS;
>+	/*
>+	 * limit check the size of rows_buf
>+	 * see MS-WSP 2.2.3.11 which describes the size
>+	 * of the rows buffer MUST not exceed 0x0004000 bytes.
>+	 * This limit will ensure we can safely check
>+	 * limits based on uint32_t offsets
>+	 */
>+
>+	if (rows_buf->length > MAX_ROW_BUFF_SIZE) {
>+		DBG_ERR("Buffer size 0x%zx exceeds 0x%x max buffer size\n",
>+			rows_buf->length, MAX_ROW_BUFF_SIZE);
>+		return NDR_ERR_BUFSIZE;
>+	}
> 
> 	for (i = 0; i < rows; i++ ) {
> 		struct wsp_cbasestoragevariant *cols =
>-- 
>2.35.3
>
>
>From 4ed6ddc2ab800253f5e965478afdb3aec8e96bab Mon Sep 17 00:00:00 2001
>From: Noel Power <noel.power@suse.com>
>Date: Thu, 8 Feb 2024 14:05:43 +0000
>Subject: [PATCH 3/3] s3/rpc_client: Fix array offset check
>
>Previous to this commit we were modifying the offset before
>the array offset check. This was causing a spurious debug
>message indicating the offset was out of bounds. An second
>problem is that upon detecting the error we don't exit the loop.
>A third problem was that when reading the offset the check
>didn't cater for the size of the integer address about to be read.
>
>This commit moves the offset check to before the first read,
>additionally when an error is detected now we actually exit the loop
>and the offset have been corrected to include the size of the
>integer to be read
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15579
>Signed-off-by: Noel Power <noel.power@suse.com>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>
>Autobuild-User(master): Volker Lendecke <vl@samba.org>
>Autobuild-Date(master): Sat Feb 17 17:58:43 UTC 2024 on atb-devel-224
>
>(cherry picked from commit 885850b6aaabf089f422b1b015481a0ccff4f90e)
>[npower@samba.org Adjusted vector processing as this function was
>    rewritten in master]
>---
> source3/rpc_client/wsp_cli.c | 54 ++++++++++++++++++++++++++++++++++--
> 1 file changed, 52 insertions(+), 2 deletions(-)
>
>diff --git a/source3/rpc_client/wsp_cli.c b/source3/rpc_client/wsp_cli.c
>index 1ce28e61807..955bbb72963 100644
>--- a/source3/rpc_client/wsp_cli.c
>+++ b/source3/rpc_client/wsp_cli.c
>@@ -939,6 +939,15 @@ static enum ndr_err_code extract_variant_addresses(TALLOC_CTX *ctx,
> 		count = 1;
> 	}
> 
>+	/* ensure count is at least within buffer range */
>+	if (count >= MAX_ROW_BUFF_SIZE || count >= rows_buf->length) {
>+		DBG_ERR("count %"PRIu64" either exceeds max buffer size "
>+			"or buffer size (%zu)",
>+			count,  rows_buf->length);
>+		err = NDR_ERR_VALIDATE;
>+		goto out;
>+	}
>+
> 	/* read address */
> 	if (is_64bit) {
> 		err = ndr_pull_udlong(ndr_pull,
>@@ -977,25 +986,66 @@ static enum ndr_err_code extract_variant_addresses(TALLOC_CTX *ctx,
> 		goto out;
> 	}
> 
>+	/*
>+	 * non vector case addr points to value
>+	 * otherwise addr points to list of addresses
>+	 * for the values in vector
>+	 */
> 	if (is_vector == false) {
> 		vec_address[0] = addr;
> 	} else {
> 		uint64_t array_addr = addr;
> 		uint64_t i;
>+		uint32_t intsize;
>+
>+		if (is_64bit) {
>+			intsize = 8;
>+		} else {
>+			intsize = 4;
>+		}
>+
>+		if (array_addr >= MAX_ROW_BUFF_SIZE
>+		    || array_addr >= rows_buf->length) {
>+			DBG_ERR("offset %"PRIu64" either exceeds max buf size "
>+				"or buffer size (%zu)",
>+				array_addr,  rows_buf->length);
>+			err = NDR_ERR_VALIDATE;
>+			goto out;
>+		}
>+
>+		/* addr points to a list of int32 or int64 addresses */
> 		for (i = 0; i < count; i++) {
>+			/*
>+			 * read the addresses of the vector elements
>+			 * note: we can safely convert the uint64_t
>+			 *       values here to uint32_t values as
>+			 *       we are sure they are within range
>+			 *       due to previous checks above.
>+			 */
>+			if (smb_buffer_oob((uint32_t)rows_buf->length,
>+					   (uint32_t)array_addr,
>+					   intsize)) {
>+				DBG_ERR("offset %"PRIu64" will be outside "
>+					"buffer range (buf len - %zu) after "
>+					"reading %s address\n",
>+					array_addr,
>+					rows_buf->length,
>+					is_64bit ? "64 bit" : "32 bit");
>+				err = NDR_ERR_VALIDATE;
>+				goto out;
>+			}
> 			if (is_64bit) {
> 				vec_address[i] =
> 					PULL_LE_I64(rows_buf->data,
> 						array_addr);
>-				array_addr = array_addr + 8;
> 			} else {
> 				vec_address[i] =
> 					(uint32_t)PULL_LE_I32(rows_buf->data,
> 							array_addr);
>-				array_addr = array_addr + 4;
> 			}
> 			/* adjust address */
> 			vec_address[i] -= offset;
>+			array_addr += intsize;
> 		}
> 	}
> 	err  = NDR_ERR_SUCCESS;
>-- 
>2.35.3
>

Flags:

vl: review-

Actions: View

Attachments on bug 15579: 18256 | 18257