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)

Autobuild-User(v4-20-test): Jule Anger <janger@samba.org>
Autobuild-Date(v4-20-test): Mon Feb 26 10:37:37 UTC 2024 on atb-devel-224

diff --git a/source3/rpc_client/wsp_cli.c b/source3/rpc_client/wsp_cli.c
index 39d1f7868d082e88320dfb6a6206250260e2d3ab..15b6e36007efc5d4255bbdebde15da902331181d 100644 (file)
--- 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;