[SCM] Samba Shared Repository - branch master updated

Volker Lendecke vlendec at samba.org
Sat Feb 17 17:59:01 UTC 2024


The branch, master has been updated
       via  885850b6aaa s3/rpc_client: Fix array offset check
       via  f487211706a s3/rpc_client: Ensure max possible row buffer size is not exceeded
       via  01e901ef869 idl: Add constant for max rows buffer size
      from  4698cf0f335 s4:dsdb: Fix grammar

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 885850b6aaabf089f422b1b015481a0ccff4f90e
Author: Noel Power <noel.power at suse.com>
Date:   Thu Feb 8 14:05:43 2024 +0000

    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 at suse.com>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    
    Autobuild-User(master): Volker Lendecke <vl at samba.org>
    Autobuild-Date(master): Sat Feb 17 17:58:43 UTC 2024 on atb-devel-224

commit f487211706a74d516bf447ed393222b4c0dce7b0
Author: Noel Power <noel.power at suse.com>
Date:   Wed Feb 14 11:19:39 2024 +0000

    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 at suse.com>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 01e901ef869a1a87fba0e67bce311dbeb199b717
Author: Noel Power <noel.power at suse.com>
Date:   Wed Feb 14 12:01:28 2024 +0000

    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 at suse.com>
    Reviewed-by: Volker Lendecke <vl at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 librpc/idl/wsp_data.idl      |  5 +++
 source3/rpc_client/wsp_cli.c | 74 ++++++++++++++++++++++++++++++++++++++------
 2 files changed, 70 insertions(+), 9 deletions(-)


Changeset truncated at 500 lines:

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";
diff --git a/source3/rpc_client/wsp_cli.c b/source3/rpc_client/wsp_cli.c
index d8a9aca46ff..15b6e36007e 100644
--- a/source3/rpc_client/wsp_cli.c
+++ b/source3/rpc_client/wsp_cli.c
@@ -938,6 +938,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,
@@ -974,30 +983,64 @@ 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_offset = addr - baseaddress;
 		uint64_t i;
+		uint32_t intsize;
+
+		if (is_64bit) {
+			intsize = 8;
+		} else {
+			intsize = 4;
+		}
+
+		if (array_offset >= MAX_ROW_BUFF_SIZE
+		    || array_offset >= rows_buf->length) {
+			DBG_ERR("offset %"PRIu64" either exceeds max buf size "
+				"or buffer size (%zu)",
+				array_offset,  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_offset,
+					   intsize)) {
+				DBG_ERR("offset %"PRIu64" will be outside "
+					"buffer range (buf len - %zu) after "
+					"reading %s address\n",
+					array_offset,
+					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_offset);
-				array_offset = array_offset + 8;
 			} else {
 				vec_address[i] =
 					(uint32_t)PULL_LE_I32(rows_buf->data,
 							array_offset);
-				array_offset = array_offset + 4;
-			}
-			if (array_offset >= rows_buf->length) {
-				DBG_ERR("offset %"PRIu64" outside buffer range "
-					"(buf len - %zu)\n",
-					array_offset,
-					rows_buf->length);
-				err = NDR_ERR_VALIDATE;
 			}
+			array_offset += intsize;
 		}
 	}
 	err  = NDR_ERR_SUCCESS;
@@ -1311,6 +1354,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 =


-- 
Samba Shared Repository



More information about the samba-cvs mailing list