[SCM] Samba Shared Repository - branch v4-20-test updated
Jule Anger
janger at samba.org
Mon Feb 26 10:38:01 UTC 2024
The branch, v4-20-test has been updated
via 253c5585c91 s3/rpc_client: Fix array offset check
via 1ab3de6f46e s3/rpc_client: Ensure max possible row buffer size is not exceeded
via 3e226dd1cd5 idl: Add constant for max rows buffer size
via c1016224041 s3/rpc_client: cleanup unmarshalling of variant types from row columns
via 77cbdf342ca s3/utils: use full 64 bit address for getrows (with 64bit offsets)
via ec239d16a97 s3/rpc_client: Remove stray unnecessary comment
via 3d47cae71d9 s3/rpc_client: change type of offset to uint64_t
from 7107b233346 ctdb-protocol: Add missing push support for new controls
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-20-test
- Log -----------------------------------------------------------------
commit 253c5585c91172ebe5cca9ca59ff30a82fbf3fd3
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
(cherry picked from commit 885850b6aaabf089f422b1b015481a0ccff4f90e)
Autobuild-User(v4-20-test): Jule Anger <janger at samba.org>
Autobuild-Date(v4-20-test): Mon Feb 26 10:37:37 UTC 2024 on atb-devel-224
commit 1ab3de6f46e61281348f9275e0ae490b53591845
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>
(cherry picked from commit f487211706a74d516bf447ed393222b4c0dce7b0)
commit 3e226dd1cd531dd070c866757e5f79492ce2b664
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>
(cherry picked from commit 01e901ef869a1a87fba0e67bce311dbeb199b717)
commit c1016224041060419f26a88e457fa8ac71e5bc12
Author: Noel Power <noel.power at suse.com>
Date: Wed Jan 10 14:43:58 2024 +0000
s3/rpc_client: cleanup unmarshalling of variant types from row columns
Prior to this change fn 'extract_variant_addresses' actually returns offsets
to the variant stored not the addresses, additionally the param in the
signature of the method is named offset where the param in reality is a
base address.
This change makes fn 'extract_variant_addresses' actually return addresses
instead of offsets and also changes the name of the incoming param. The
resulting changes are propaged to callers which hopefully makes what the
code is actually doing a little clearer
Signed-off-by: Noel Power <noel.power at suse.com>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
Autobuild-User(master): Noel Power <npower at samba.org>
Autobuild-Date(master): Tue Jan 30 17:22:37 UTC 2024 on atb-devel-224
(cherry picked from commit 9b2f2302ee4828ae54f5903a3bf649ffd255fb4a)
commit 77cbdf342ca05a8f21c316e58395576e954d857b
Author: Noel Power <noel.power at suse.com>
Date: Mon Jan 8 15:56:38 2024 +0000
s3/utils: use full 64 bit address for getrows (with 64bit offsets)
if 64bit offsets are used the hi 32-bits of address are stored in
the ulreserved2 member of the message header field and the low 32-bits
are stored in the ulclientbase member of the cpmgetrows message
Signed-off-by: Noel Power <noel.power at suse.com>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
(cherry picked from commit 6ecb614b8ec6953ba15e8061fce9b395615b035a)
commit ec239d16a970daae26acadb0c4a732e349e3435d
Author: Noel Power <noel.power at suse.com>
Date: Wed Jan 10 10:59:23 2024 +0000
s3/rpc_client: Remove stray unnecessary comment
Signed-off-by: Noel Power <noel.power at suse.com>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
(cherry picked from commit efa60ff3105ac80ffff6d2a5d82dd0615ddb7578)
commit 3d47cae71d953e05e793ca5dd392fa6e260e23e0
Author: Noel Power <noel.power at suse.com>
Date: Mon Jan 8 15:12:35 2024 +0000
s3/rpc_client: change type of offset to uint64_t
Offset can be a 32 or 64 bit address depending on the indexing addressing
mode negotiated by the client
With a 32 bit param we can only specify a 32 bit base address. This change
alone doesn't affect anything as it is the client itself that choses and
passes the base address offset and wspsearch is the only current user of
this code.
In this case even with 64bit addressing negotiated the address passed
represents only the lower 32-bits part of the address.
However, for coverage purposes it would be better for the client to use an
address that covers the full 64bit range of the address (when 64 bit
addressing is negotiated).
This change will alow the wspsearch client in a future commit to pass a
base address value with both the hi and low 32 bits values set to make up
the full 64 bit address.
Signed-off-by: Noel Power <noel.power at suse.com>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
(cherry picked from commit a61eb7032896265eaef3ba225aafd6f293e7569d)
-----------------------------------------------------------------------
Summary of changes:
librpc/idl/wsp_data.idl | 5 ++
source3/rpc_client/wsp_cli.c | 127 ++++++++++++++++++++++++++++++++-----------
source3/utils/wspsearch.c | 22 +++++++-
3 files changed, 120 insertions(+), 34 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 b21f55c86f0..15b6e36007e 100644
--- a/source3/rpc_client/wsp_cli.c
+++ b/source3/rpc_client/wsp_cli.c
@@ -761,7 +761,7 @@ void create_seekat_getrows_request(TALLOC_CTX * ctx,
static bool extract_rowbuf_variable_type(TALLOC_CTX *ctx,
uint16_t type,
- uint32_t offset,
+ uint64_t offset,
DATA_BLOB *rows_buf, uint32_t len,
struct wsp_cbasestoragevariant *val)
{
@@ -770,7 +770,7 @@ static bool extract_rowbuf_variable_type(TALLOC_CTX *ctx,
ndr_flags_type ndr_flags = NDR_SCALARS | NDR_BUFFERS;
DATA_BLOB variant_blob = data_blob_null;
if (offset >= rows_buf->length) {
- DBG_ERR("offset %d outside buffer range (buf len - %zu)",
+ DBG_ERR("offset %"PRIu64" outside buffer range (buf len - %zu)",
offset,
rows_buf->length);
return false;
@@ -894,7 +894,7 @@ static bool convert_variant_array_to_vector(TALLOC_CTX *ctx,
* an array of n elements for a vector or array of 1 element
* if non-vector item.
*
- * addresses stored in pvec_address are adjusted by offset
+ * addresses stored in pvec_address
*
*/
static enum ndr_err_code extract_variant_addresses(TALLOC_CTX *ctx,
@@ -902,11 +902,10 @@ static enum ndr_err_code extract_variant_addresses(TALLOC_CTX *ctx,
bool is_64bit,
struct ndr_pull *ndr_pull,
ndr_flags_type flags,
- uint32_t offset,
+ uint64_t baseaddress,
DATA_BLOB *rows_buf,
uint64_t *pcount,
- uint64_t **pvec_address/*,
- struct wsp_cbasestoragevariant ***variant_array*/)
+ uint64_t **pvec_address)
{
bool is_vector = tablevar->vtype & VT_VECTOR;
uint64_t count;
@@ -939,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,
@@ -958,12 +966,10 @@ static enum ndr_err_code extract_variant_addresses(TALLOC_CTX *ctx,
addr = addr_32;
}
- addr = addr - offset;
-
- if (addr >= rows_buf->length) {
+ if ((addr - baseaddress) >= rows_buf->length) {
DBG_ERR("offset %"PRIu64" outside buffer range "
"(buf len - %zu)\n",
- addr,
+ addr - baseaddress,
rows_buf->length);
err = NDR_ERR_VALIDATE;
goto out;
@@ -977,25 +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_addr = addr;
+ 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_addr);
- array_addr = array_addr + 8;
+ array_offset);
} else {
vec_address[i] =
(uint32_t)PULL_LE_I32(rows_buf->data,
- array_addr);
- array_addr = array_addr + 4;
+ array_offset);
}
- /* adjust address */
- vec_address[i] -= offset;
+ array_offset += intsize;
}
}
err = NDR_ERR_SUCCESS;
@@ -1010,7 +1055,7 @@ static enum ndr_err_code extract_crowvariant_variable(TALLOC_CTX *ctx,
bool is_64bit,
struct ndr_pull *ndr_pull,
ndr_flags_type flags,
- uint32_t offset,
+ uint64_t baseaddress,
DATA_BLOB *rows_buf,
uint32_t len,
struct wsp_cbasestoragevariant *val)
@@ -1029,7 +1074,7 @@ static enum ndr_err_code extract_crowvariant_variable(TALLOC_CTX *ctx,
is_64bit,
ndr_pull,
flags,
- offset,
+ baseaddress,
rows_buf,
&count,
&vec_address);
@@ -1063,12 +1108,12 @@ static enum ndr_err_code extract_crowvariant_variable(TALLOC_CTX *ctx,
for (i = 0; i < count; i++) {
uint32_t tmplen = len;
- uint64_t addr;
- addr = vec_address[i];
- if (addr >= rows_buf->length) {
+ uint64_t buf_offset;
+ buf_offset = vec_address[i] - baseaddress;
+ if (buf_offset >= rows_buf->length) {
DBG_ERR("offset %"PRIu64" outside buffer range "
"(buf len - %zu)\n",
- addr,
+ buf_offset,
rows_buf->length);
err = NDR_ERR_VALIDATE;
goto out;
@@ -1084,11 +1129,11 @@ static enum ndr_err_code extract_crowvariant_variable(TALLOC_CTX *ctx,
* from the point the value is stored at
* till the end of the buffer
*/
- tmplen = rows_buf->length - addr;
+ tmplen = rows_buf->length - buf_offset;
}
if (!extract_rowbuf_variable_type(ctx,
tablevar->vtype & ~VT_VECTOR,
- addr,
+ buf_offset,
rows_buf,
tmplen,
variant_array[i])) {
@@ -1116,7 +1161,7 @@ static enum ndr_err_code extract_crowvariant(TALLOC_CTX *ctx,
bool is_64bit,
struct ndr_pull *ndr_pull,
ndr_flags_type flags,
- uint32_t offset,
+ uint64_t baseaddress,
DATA_BLOB *rows_buf, uint32_t len,
struct wsp_cbasestoragevariant *val)
{
@@ -1136,7 +1181,7 @@ static enum ndr_err_code extract_crowvariant(TALLOC_CTX *ctx,
is_64bit,
ndr_pull,
flags,
- offset,
+ baseaddress,
rows_buf,
len,
val);
@@ -1159,7 +1204,6 @@ out:
static enum ndr_err_code process_columns(TALLOC_CTX *ctx,
bool is_64bit,
- uint32_t cbreserved,
uint64_t baseaddress,
struct wsp_cpmsetbindingsin *bindingin,
DATA_BLOB *rows_buf,
@@ -1225,7 +1269,6 @@ static enum ndr_err_code process_columns(TALLOC_CTX *ctx,
val_offset));
}
if (tab_col->valueused) {
- uint64_t offset = baseaddress + cbreserved;
uint32_t len = 0;
val_offset = nrow_offset + tab_col->valueoffset.value;
if (val_offset >= rows_buf->length) {
@@ -1285,7 +1328,7 @@ static enum ndr_err_code process_columns(TALLOC_CTX *ctx,
is_64bit,
ndr_pull,
ndr_flags,
- offset,
+ baseaddress,
rows_buf,
len,
&cols[i]);
@@ -1311,19 +1354,39 @@ 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 =
talloc_zero_array(ctx,
struct wsp_cbasestoragevariant,
bindingsin->ccolumns);
+ uint64_t adjusted_address;
if (cols == NULL) {
return NDR_ERR_ALLOC;
}
+
+ /*
+ * cater for paddingrows (see MS-WSP 2.2.3.12)
+ * Rows buffer starts cbreserved bytes into messages
+ */
+ adjusted_address = baseaddress + cbreserved;
+
err = process_columns(ctx,
is_64bit,
- cbreserved,
- baseaddress,
+ adjusted_address,
bindingsin,
rows_buf,
i,
diff --git a/source3/utils/wspsearch.c b/source3/utils/wspsearch.c
index 2c56c97736b..063b952d468 100644
--- a/source3/utils/wspsearch.c
+++ b/source3/utils/wspsearch.c
@@ -350,6 +350,10 @@ static NTSTATUS create_getrows(TALLOC_CTX *ctx,
uint32_t INITIAL_ROWS = 32;
uint32_t requested_rows = INITIAL_ROWS;
uint32_t rows_printed;
+ uint64_t baseaddress;
+ uint32_t offset_lowbits = 0xdeabd860;
+ uint32_t offset_hibits = 0xfeeddeaf;
+
TALLOC_CTX *row_ctx;
bool loop_again;
@@ -377,10 +381,24 @@ static NTSTATUS create_getrows(TALLOC_CTX *ctx,
skip,
requested_rows,
40,
- 0xDEAbd860,
+ offset_lowbits,
bindings->brow,
0);
+ if (is_64bit) {
+ /*
+ * MS-WSP 2.2.2
+ * ulreservered holds the high 32-bits part of
+ * a 64-bit offset if 64-bit offsets are being used.
+ */
+ request->header.ulreserved2 = offset_hibits;
+ baseaddress = request->header.ulreserved2;
+ baseaddress <<= 32;
+ baseaddress += offset_lowbits;
+ } else {
+ baseaddress = offset_lowbits;
+ }
+
status = wsp_request_response(request,
wsp_ctx,
request,
@@ -419,7 +437,7 @@ static NTSTATUS create_getrows(TALLOC_CTX *ctx,
is_64bit,
disp_all_cols,
bindings, 40,
- 0xDEAbd860,
+ baseaddress,
response->message.cpmgetrows.rowsreturned,
&rows_printed);
if (!NT_STATUS_IS_OK(status)) {
--
Samba Shared Repository
More information about the samba-cvs
mailing list