[SCM] Samba Shared Repository - branch master updated

Andrew Bartlett abartlet at samba.org
Thu Dec 12 03:57:02 UTC 2019


The branch, master has been updated
       via  82aff583b7f libndr: Return enum ndr_err_code from ndr_{pull,push}_steal_switch_value()
       via  d30c377d8c4 librpc/ndr: Remove ndr_{push,pull}_get_switch_value()
       via  a44717394df librpc: Remove last callers of ndr_push_get_switch_value()
       via  b880bda589a librpc: Remove last callers of ndr_pull_get_switch_value()
       via  92a7c5a7262 librpc: Do not follow a NULL pointer when calculating the size of a union
       via  456cdb7fa35 pidl: Mismatch between set and get of relative base pointers
       via  362d70ff2fb librpc: Do not follow a NULL pointer when calculating the size of a structure
       via  5eb560d25e9 pidl: Mismatch between set and get of relative base pointers
       via  e08461b2845 pidl: Add and use ndr_print_steal_switch_value(), removing ndr_print_get_switch_value()
       via  e1a15710a4c librpc/ndr: Add ndr_push_steal_switch_value()
       via  603f23b19c9 pidl: Generate and consume the switch level token for both NDR_SCALARS and NDR_BUFFERS in ndr_pull()
       via  f7bcf227f7b librpc: Set the switch_value before NDR_BUFFERS to prepare for new libndr behaviour
       via  a22a22e4a8a negoex: Set the switch_value before NDR_BUFFERS to prepare for new libndr behaviour
       via  ae43093b79a s4-libcli/rap: Set the switch_value before NDR_BUFFERS to prepare for new libndr behaviour
       via  7a0ed44b0e6 ndr: Restrict size of ndr_token lists to avoid memory abuse by malicious clients
       via  4501663f6e5 libndr: Do not overwrite token list with NULL on allocation failure
       via  bcffdc9a895 selftest: Add test for ndr_size_struct() faulting on a NULL pointer
       via  f56fa3bb6a0 selftest: Add test for ndr_size_union() faulting on a NULL pointer
       via  6ef50145494 selftest: Add example xattr_NTACL packets to demonstrate switch/union behaviour
       via  cf83eec565f selftest: Add test for structure with NDR_BUFFERS only in a union
      from  3f69c6b1328 selftest: Do not force the endpoint for fsrvp tests

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


- Log -----------------------------------------------------------------
commit 82aff583b7f7e018ad4a1db92dc635df8e5ebe7b
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed Dec 4 12:45:42 2019 +1300

    libndr: Return enum ndr_err_code from ndr_{pull,push}_steal_switch_value()
    
    This breaks the ABI so we merge this into the unreleased libndr-1.0.0.
    
    The advantage of the new functions is there (except for print, which
    is unchanged) is an error raised when the token is not found, so
    we can be confident in the changes to the token behaviour.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>
    
    Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
    Autobuild-Date(master): Thu Dec 12 03:56:23 UTC 2019 on sn-devel-184

commit d30c377d8c4e8a83b2722b771aa72d219eb69b46
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed Nov 27 15:20:32 2019 +1300

    librpc/ndr: Remove ndr_{push,pull}_get_switch_value()
    
    By removing this we know we do not need to worry about this list
    growing without bounds.  We merge this into the recently created but
    not yet released ABI 1.0.0
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit a44717394df9dc14a87eb2b9b8d23e2532e6f4c9
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed Nov 27 16:36:04 2019 +1300

    librpc: Remove last callers of ndr_push_get_switch_value()
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit b880bda589aa136bb954a757f25f8763b74aefd4
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Nov 26 18:35:36 2019 +1300

    librpc: Remove last callers of ndr_pull_get_switch_value()
    
    By removing this we know we do not need to worry about this list
    growing without bounds.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 92a7c5a7262cb78fb6307e9168a540154cbe2f71
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Nov 19 17:38:50 2019 +1300

    librpc: Do not follow a NULL pointer when calculating the size of a union
    
    Found by Douglas Bagnall using Hongfuzz and the new fuzz_ndr_X
    fuzzer.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 456cdb7fa35f2b5778c7a5c852acc6180cbe03b4
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Nov 15 20:04:41 2019 +1300

    pidl: Mismatch between set and get of relative base pointers
    
    The set was within the switch, the get was before the switch.
    
    The difference is shown when there is an empty default element.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 362d70ff2fb6fb6265fce03da6b09dd4756dc604
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Nov 19 16:58:57 2019 +1300

    librpc: Do not follow a NULL pointer when calculating the size of a structure
    
    Found by Douglas Bagnall using Hongfuzz and the new fuzz_ndr_X
    fuzzer.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 5eb560d25e9104dc02477a6bf819d0c37e8afb86
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Nov 15 20:04:41 2019 +1300

    pidl: Mismatch between set and get of relative base pointers
    
    The set was within the switch, the get was before the switch.
    
    The difference is shown when there is an empty default element.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit e08461b2845183224b9775b54be4acaaba705ecd
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Nov 18 12:02:03 2019 +1300

    pidl: Add and use ndr_print_steal_switch_value(), removing ndr_print_get_switch_value()
    
    This avoids really long token lists for switch values
    that will not be needed past this point.
    
    The function name is changed to clarify what exactly is being
    done here, and the old function is removed to ensure it is
    not being used anywhere else.
    
    Merge the removal of ndr_print_get_switch_value into
    just-tagged librpc/ABI/ndr-1.0.0.sigs as this
    has not been put into any release yet.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit e1a15710a4c1c96c9d9fdf932357749cc5ed4d7a
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed Nov 27 16:01:02 2019 +1300

    librpc/ndr: Add ndr_push_steal_switch_value()
    
    This will allow generated code to instead push and pop union values onto the
    switch_list stack, which is more memory efficient than creating a single large
    list to be scannned and eventually discarded.
    
    Merge into unreleased ABI 1.0.0
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 603f23b19c90d6a1be4ac50e5984e6f9c8a63ba2
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Nov 15 16:59:12 2019 +1300

    pidl: Generate and consume the switch level token for both NDR_SCALARS and NDR_BUFFERS in ndr_pull()
    
    This means what was previously a list becomes a single variable that
    could be passed as a function paraemter, but this is avoided for now
    because it would change the ABI and be more intrusive.
    
    Before this, a client could cause a NDR token containing the swith level
    to be allocated for each and every element in the array that they
    promised they were sending (without having to actually send them).
    
    Found by Michael Hanselmann using Honggfuzz and an fuzzer for Samba's
    NDR layer.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit f7bcf227f7bac1817ecabbe3dccaebdd9a81a2f9
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed Dec 4 15:30:55 2019 +1300

    librpc: Set the switch_value before NDR_BUFFERS to prepare for new libndr behaviour
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit a22a22e4a8a2efad86833d21958bc6abf57f74e7
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Sun Nov 17 19:32:50 2019 +1300

    negoex: Set the switch_value before NDR_BUFFERS to prepare for new libndr behaviour
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit ae43093b79ad59a090e72c22249a65b54a2a6a97
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Sat Nov 16 09:37:30 2019 +1300

    s4-libcli/rap: Set the switch_value before NDR_BUFFERS to prepare for new libndr behaviour
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 7a0ed44b0e65e742a778915d493e17f04c43b2ef
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Sat Nov 16 07:59:58 2019 +1300

    ndr: Restrict size of ndr_token lists to avoid memory abuse by malicious clients
    
    This is designed to stop a very large number of tokens from being stored for
    arrays of structures containing relative pointers in particular.
    
    This was one part of the minimum patch for CVE-2019-14908 before
    being downgraded as not a security-release worthy issue.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 4501663f6e523a988f4edd287a1374cab3c9a127
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Nov 18 10:38:01 2019 +1300

    libndr: Do not overwrite token list with NULL on allocation failure
    
    This was one part of the minimum patch for CVE-2019-14908 before
    being downgraded as not a security-release worthy issue.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit bcffdc9a895c8aa572819ddd4fca451038990402
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed Dec 4 16:56:44 2019 +1300

    selftest: Add test for ndr_size_struct() faulting on a NULL pointer
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit f56fa3bb6a0e2275114a6f4dd27640af1e15018b
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed Dec 4 16:49:13 2019 +1300

    selftest: Add test for ndr_size_union() faulting on a NULL pointer
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 6ef50145494125c8e67c034ddb1816a567524984
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed Dec 4 16:02:31 2019 +1300

    selftest: Add example xattr_NTACL packets to demonstrate switch/union behaviour
    
    This is a good example with both buffers and scalars in the union.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit cf83eec565f2879c06874cdd390ab3a043c132f8
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed Dec 4 14:46:32 2019 +1300

    selftest: Add test for structure with NDR_BUFFERS only in a union
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

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

Summary of changes:
 librpc/ABI/ndr-1.0.0.sigs                          |   7 +-
 librpc/ndr/libndr.h                                |  14 +-
 librpc/ndr/ndr.c                                   | 169 +++++++++++++++++----
 librpc/ndr/ndr_drsuapi.c                           |  48 +-----
 librpc/ndr/ndr_negoex.c                            |   1 +
 librpc/ndr/ndr_ntlmssp.c                           |   2 +-
 librpc/ndr/ndr_schannel.c                          |   4 +-
 librpc/ndr/ndr_sec_helper.c                        |   1 +
 librpc/ndr/ndr_spoolss_buf.c                       |   2 +-
 pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm           |  65 +++++---
 python/samba/tests/blackbox/ndrdump.py             |  99 ++++++++++++
 source4/libcli/rap/rap.c                           |   3 +
 .../tests/fuzzed_ntlmssp-CHALLENGE_MESSAGE.txt     |  43 ++++++
 source4/librpc/tests/xattr_NTACL.dat               |  20 +++
 source4/librpc/tests/xattr_NTACL.txt               |  99 ++++++++++++
 15 files changed, 468 insertions(+), 109 deletions(-)
 create mode 100644 source4/librpc/tests/fuzzed_ntlmssp-CHALLENGE_MESSAGE.txt
 create mode 100644 source4/librpc/tests/xattr_NTACL.dat
 create mode 100644 source4/librpc/tests/xattr_NTACL.txt


Changeset truncated at 500 lines:

diff --git a/librpc/ABI/ndr-1.0.0.sigs b/librpc/ABI/ndr-1.0.0.sigs
index ffb3d0763fc..bc7c3e88848 100644
--- a/librpc/ABI/ndr-1.0.0.sigs
+++ b/librpc/ABI/ndr-1.0.0.sigs
@@ -49,7 +49,6 @@ ndr_print_double: void (struct ndr_print *, const char *, double)
 ndr_print_enum: void (struct ndr_print *, const char *, const char *, const char *, uint32_t)
 ndr_print_function_debug: void (ndr_print_function_t, const char *, int, void *)
 ndr_print_function_string: char *(TALLOC_CTX *, ndr_print_function_t, const char *, int, void *)
-ndr_print_get_switch_value: uint32_t (struct ndr_print *, const void *)
 ndr_print_gid_t: void (struct ndr_print *, const char *, gid_t)
 ndr_print_hyper: void (struct ndr_print *, const char *, uint64_t)
 ndr_print_int16: void (struct ndr_print *, const char *, int16_t)
@@ -68,6 +67,7 @@ ndr_print_printf_helper: void (struct ndr_print *, const char *, ...)
 ndr_print_ptr: void (struct ndr_print *, const char *, const void *)
 ndr_print_set_switch_value: enum ndr_err_code (struct ndr_print *, const void *, uint32_t)
 ndr_print_sockaddr_storage: void (struct ndr_print *, const char *, const struct sockaddr_storage *)
+ndr_print_steal_switch_value: uint32_t (struct ndr_print *, const void *)
 ndr_print_string: void (struct ndr_print *, const char *, const char *)
 ndr_print_string_array: void (struct ndr_print *, const char *, const char **)
 ndr_print_string_helper: void (struct ndr_print *, const char *, ...)
@@ -116,7 +116,6 @@ ndr_pull_enum_uint32: enum ndr_err_code (struct ndr_pull *, int, uint32_t *)
 ndr_pull_enum_uint8: enum ndr_err_code (struct ndr_pull *, int, uint8_t *)
 ndr_pull_generic_ptr: enum ndr_err_code (struct ndr_pull *, uint32_t *)
 ndr_pull_get_relative_base_offset: uint32_t (struct ndr_pull *)
-ndr_pull_get_switch_value: uint32_t (struct ndr_pull *, const void *)
 ndr_pull_gid_t: enum ndr_err_code (struct ndr_pull *, int, gid_t *)
 ndr_pull_hyper: enum ndr_err_code (struct ndr_pull *, int, uint64_t *)
 ndr_pull_init_blob: struct ndr_pull *(const DATA_BLOB *, TALLOC_CTX *)
@@ -139,7 +138,7 @@ ndr_pull_restore_relative_base_offset: void (struct ndr_pull *, uint32_t)
 ndr_pull_set_switch_value: enum ndr_err_code (struct ndr_pull *, const void *, uint32_t)
 ndr_pull_setup_relative_base_offset1: enum ndr_err_code (struct ndr_pull *, const void *, uint32_t)
 ndr_pull_setup_relative_base_offset2: enum ndr_err_code (struct ndr_pull *, const void *)
-ndr_pull_steal_switch_value: uint32_t (struct ndr_pull *, const void *)
+ndr_pull_steal_switch_value: enum ndr_err_code (struct ndr_pull *, const void *, uint32_t *)
 ndr_pull_string: enum ndr_err_code (struct ndr_pull *, int, const char **)
 ndr_pull_string_array: enum ndr_err_code (struct ndr_pull *, int, const char ***)
 ndr_pull_struct_blob: enum ndr_err_code (const DATA_BLOB *, TALLOC_CTX *, void *, ndr_pull_flags_fn_t)
@@ -190,7 +189,6 @@ ndr_push_enum_uint8: enum ndr_err_code (struct ndr_push *, int, uint8_t)
 ndr_push_expand: enum ndr_err_code (struct ndr_push *, uint32_t)
 ndr_push_full_ptr: enum ndr_err_code (struct ndr_push *, const void *)
 ndr_push_get_relative_base_offset: uint32_t (struct ndr_push *)
-ndr_push_get_switch_value: uint32_t (struct ndr_push *, const void *)
 ndr_push_gid_t: enum ndr_err_code (struct ndr_push *, int, gid_t)
 ndr_push_hyper: enum ndr_err_code (struct ndr_push *, int, uint64_t)
 ndr_push_init_ctx: struct ndr_push *(TALLOC_CTX *)
@@ -215,6 +213,7 @@ ndr_push_setup_relative_base_offset1: enum ndr_err_code (struct ndr_push *, cons
 ndr_push_setup_relative_base_offset2: enum ndr_err_code (struct ndr_push *, const void *)
 ndr_push_short_relative_ptr1: enum ndr_err_code (struct ndr_push *, const void *)
 ndr_push_short_relative_ptr2: enum ndr_err_code (struct ndr_push *, const void *)
+ndr_push_steal_switch_value: enum ndr_err_code (struct ndr_push *, const void *, uint32_t *)
 ndr_push_string: enum ndr_err_code (struct ndr_push *, int, const char *)
 ndr_push_string_array: enum ndr_err_code (struct ndr_push *, int, const char **)
 ndr_push_struct_blob: enum ndr_err_code (DATA_BLOB *, TALLOC_CTX *, const void *, ndr_push_flags_fn_t)
diff --git a/librpc/ndr/libndr.h b/librpc/ndr/libndr.h
index 392ab76cc94..58ef517d363 100644
--- a/librpc/ndr/libndr.h
+++ b/librpc/ndr/libndr.h
@@ -616,10 +616,16 @@ enum ndr_err_code ndr_check_pipe_chunk_trailer(struct ndr_pull *ndr, int ndr_fla
 enum ndr_err_code ndr_push_set_switch_value(struct ndr_push *ndr, const void *p, uint32_t val);
 enum ndr_err_code ndr_pull_set_switch_value(struct ndr_pull *ndr, const void *p, uint32_t val);
 enum ndr_err_code ndr_print_set_switch_value(struct ndr_print *ndr, const void *p, uint32_t val);
-uint32_t ndr_push_get_switch_value(struct ndr_push *ndr, const void *p);
-uint32_t ndr_pull_get_switch_value(struct ndr_pull *ndr, const void *p);
-uint32_t ndr_print_get_switch_value(struct ndr_print *ndr, const void *p);
-uint32_t ndr_pull_steal_switch_value(struct ndr_pull *ndr, const void *p);
+/* retrieve a switch value (for push) and remove it from the list */
+enum ndr_err_code ndr_push_steal_switch_value(struct ndr_push *ndr,
+					      const void *p,
+					      uint32_t *v);
+/* retrieve a switch value and remove it from the list */
+uint32_t ndr_print_steal_switch_value(struct ndr_print *ndr, const void *p);
+/* retrieve a switch value and remove it from the list */
+enum ndr_err_code ndr_pull_steal_switch_value(struct ndr_pull *ndr,
+					      const void *p,
+					      uint32_t *v);
 enum ndr_err_code ndr_pull_struct_blob(const DATA_BLOB *blob, TALLOC_CTX *mem_ctx, void *p, ndr_pull_flags_fn_t fn);
 enum ndr_err_code ndr_pull_struct_blob_all(const DATA_BLOB *blob, TALLOC_CTX *mem_ctx, void *p, ndr_pull_flags_fn_t fn);
 enum ndr_err_code ndr_pull_struct_blob_all_noalloc(const DATA_BLOB *blob,
diff --git a/librpc/ndr/ndr.c b/librpc/ndr/ndr.c
index 53f9a816f94..2259a35b170 100644
--- a/librpc/ndr/ndr.c
+++ b/librpc/ndr/ndr.c
@@ -36,6 +36,17 @@
 
 #define NDR_BASE_MARSHALL_SIZE 1024
 
+/*
+ * This value is arbitary, but designed to reduce the memory a client
+ * can allocate and the work the client can force in processing a
+ * malicious packet.
+ *
+ * In an ideal world this would be controlled by range() restrictions
+ * on array sizes and careful IDL construction to avoid arbitary
+ * linked lists, but this is a backstop for now.
+ */
+#define NDR_TOKEN_MAX_LIST_SIZE 65535
+
 /* this guid indicates NDR encoding in a protocol tower */
 const struct ndr_syntax_id ndr_transfer_syntax_ndr = {
   { 0x8a885d04, 0x1ceb, 0x11c9, {0x9f, 0xe8}, {0x08,0x00,0x2b,0x10,0x48,0x60} },
@@ -972,19 +983,32 @@ _PUBLIC_ enum ndr_err_code ndr_token_store(TALLOC_CTX *mem_ctx,
 			NDR_ERR_HAVE_NO_MEMORY(list->tokens);
 		}
 	} else {
+		struct ndr_token *new_tokens = NULL;
 		uint32_t alloc_count = talloc_array_length(list->tokens);
+
+		/*
+		 * Check every time we have not allocated too many
+		 * tokens.  This ensures developer sanity when
+		 * debugging the boundary condition
+		 */
+		if (list->count >= NDR_TOKEN_MAX_LIST_SIZE) {
+			return NDR_ERR_RANGE;
+		}
 		if (list->count == alloc_count) {
 			unsigned new_alloc;
+			/*
+			 * Double the list, until we start in chunks
+			 * of 1000
+			 */
 			unsigned increment = MIN(list->count, 1000);
 			new_alloc = alloc_count + increment;
 			if (new_alloc < alloc_count) {
 				return NDR_ERR_RANGE;
 			}
-			list->tokens = talloc_realloc(mem_ctx, list->tokens,
-						      struct ndr_token, new_alloc);
-			if (list->tokens == NULL) {
-				NDR_ERR_HAVE_NO_MEMORY(list->tokens);
-			}
+			new_tokens = talloc_realloc(mem_ctx, list->tokens,
+						    struct ndr_token, new_alloc);
+			NDR_ERR_HAVE_NO_MEMORY(new_tokens);
+			list->tokens = new_tokens;
 		}
 	}
 	list->tokens[list->count].key = key;
@@ -1059,9 +1083,16 @@ _PUBLIC_ uint32_t ndr_token_peek(struct ndr_token_list *list, const void *key)
 */
 _PUBLIC_ enum ndr_err_code ndr_pull_array_size(struct ndr_pull *ndr, const void *p)
 {
+	enum ndr_err_code ret;
 	uint32_t size;
 	NDR_CHECK(ndr_pull_uint3264(ndr, NDR_SCALARS, &size));
-	return ndr_token_store(ndr, &ndr->array_size_list, p, size);
+	ret = ndr_token_store(ndr, &ndr->array_size_list, p, size);
+	if (ret == NDR_ERR_RANGE) {
+		return ndr_pull_error(ndr, ret,
+				      "More than %d NDR tokens stored for array_size",
+				      NDR_TOKEN_MAX_LIST_SIZE);
+	}
+	return ret;
 }
 
 /*
@@ -1092,6 +1123,7 @@ _PUBLIC_ enum ndr_err_code ndr_check_array_size(struct ndr_pull *ndr, void *p, u
 */
 _PUBLIC_ enum ndr_err_code ndr_pull_array_length(struct ndr_pull *ndr, const void *p)
 {
+	enum ndr_err_code ret;
 	uint32_t length, offset;
 	NDR_CHECK(ndr_pull_uint3264(ndr, NDR_SCALARS, &offset));
 	if (offset != 0) {
@@ -1099,7 +1131,13 @@ _PUBLIC_ enum ndr_err_code ndr_pull_array_length(struct ndr_pull *ndr, const voi
 				      "non-zero array offset %u\n", offset);
 	}
 	NDR_CHECK(ndr_pull_uint3264(ndr, NDR_SCALARS, &length));
-	return ndr_token_store(ndr, &ndr->array_length_list, p, length);
+	ret = ndr_token_store(ndr, &ndr->array_length_list, p, length);
+	if (ret == NDR_ERR_RANGE) {
+		return ndr_pull_error(ndr, ret,
+				      "More than %d NDR tokens stored for array_length_list",
+				      NDR_TOKEN_MAX_LIST_SIZE);
+	}
+	return ret;
 }
 
 /*
@@ -1164,12 +1202,27 @@ _PUBLIC_ enum ndr_err_code ndr_check_pipe_chunk_trailer(struct ndr_pull *ndr, in
  */
 _PUBLIC_ enum ndr_err_code ndr_push_set_switch_value(struct ndr_push *ndr, const void *p, uint32_t val)
 {
-	return ndr_token_store(ndr, &ndr->switch_list, p, val);
+	enum ndr_err_code ret =
+		ndr_token_store(ndr, &ndr->switch_list, p, val);
+	if (ret == NDR_ERR_RANGE) {
+		return ndr_push_error(ndr, ret,
+				      "More than %d NDR tokens stored for switch_list",
+				      NDR_TOKEN_MAX_LIST_SIZE);
+	}
+	return ret;
 }
 
 _PUBLIC_ enum ndr_err_code ndr_pull_set_switch_value(struct ndr_pull *ndr, const void *p, uint32_t val)
 {
-	return ndr_token_store(ndr, &ndr->switch_list, p, val);
+
+	enum ndr_err_code ret =
+		ndr_token_store(ndr, &ndr->switch_list, p, val);
+	if (ret == NDR_ERR_RANGE) {
+		return ndr_pull_error(ndr, ret,
+				      "More than %d NDR tokens stored for switch_list",
+				      NDR_TOKEN_MAX_LIST_SIZE);
+	}
+	return ret;
 }
 
 _PUBLIC_ enum ndr_err_code ndr_print_set_switch_value(struct ndr_print *ndr, const void *p, uint32_t val)
@@ -1177,26 +1230,16 @@ _PUBLIC_ enum ndr_err_code ndr_print_set_switch_value(struct ndr_print *ndr, con
 	return ndr_token_store(ndr, &ndr->switch_list, p, val);
 }
 
-/*
-  retrieve a switch value
- */
-_PUBLIC_ uint32_t ndr_push_get_switch_value(struct ndr_push *ndr, const void *p)
-{
-	return ndr_token_peek(&ndr->switch_list, p);
-}
-
-_PUBLIC_ uint32_t ndr_pull_get_switch_value(struct ndr_pull *ndr, const void *p)
-{
-	return ndr_token_peek(&ndr->switch_list, p);
-}
-
-_PUBLIC_ uint32_t ndr_print_get_switch_value(struct ndr_print *ndr, const void *p)
+/* retrieve a switch value (for push) and remove it from the list */
+_PUBLIC_ enum ndr_err_code ndr_push_steal_switch_value(struct ndr_push *ndr,
+						       const void *p,
+						       uint32_t *v)
 {
-	return ndr_token_peek(&ndr->switch_list, p);
+	return ndr_token_retrieve(&ndr->switch_list, p, v);
 }
 
 /* retrieve a switch value and remove it from the list */
-_PUBLIC_ uint32_t ndr_pull_steal_switch_value(struct ndr_pull *ndr, const void *p)
+_PUBLIC_ uint32_t ndr_print_steal_switch_value(struct ndr_print *ndr, const void *p)
 {
 	enum ndr_err_code status;
 	uint32_t v;
@@ -1209,6 +1252,14 @@ _PUBLIC_ uint32_t ndr_pull_steal_switch_value(struct ndr_pull *ndr, const void *
 	return v;
 }
 
+/* retrieve a switch value and remove it from the list */
+_PUBLIC_ enum ndr_err_code ndr_pull_steal_switch_value(struct ndr_pull *ndr,
+						       const void *p,
+						       uint32_t *v)
+{
+	return ndr_token_retrieve(&ndr->switch_list, p, v);
+}
+
 /*
   pull a struct from a blob using NDR
 */
@@ -1416,6 +1467,11 @@ _PUBLIC_ size_t ndr_size_struct(const void *p, int flags, ndr_push_flags_fn_t pu
 	/* avoid recursion */
 	if (flags & LIBNDR_FLAG_NO_NDR_SIZE) return 0;
 
+	/* Avoid following a NULL pointer */
+	if (p == NULL) {
+		return 0;
+	}
+
 	ndr = ndr_push_init_ctx(NULL);
 	if (!ndr) return 0;
 	ndr->flags |= flags | LIBNDR_FLAG_NO_NDR_SIZE;
@@ -1441,6 +1497,11 @@ _PUBLIC_ size_t ndr_size_union(const void *p, int flags, uint32_t level, ndr_pus
 	/* avoid recursion */
 	if (flags & LIBNDR_FLAG_NO_NDR_SIZE) return 0;
 
+	/* Avoid following a NULL pointer */
+	if (p == NULL) {
+		return 0;
+	}
+
 	ndr = ndr_push_init_ctx(NULL);
 	if (!ndr) return 0;
 	ndr->flags |= flags | LIBNDR_FLAG_NO_NDR_SIZE;
@@ -1482,8 +1543,15 @@ _PUBLIC_ void ndr_push_restore_relative_base_offset(struct ndr_push *ndr, uint32
 */
 _PUBLIC_ enum ndr_err_code ndr_push_setup_relative_base_offset1(struct ndr_push *ndr, const void *p, uint32_t offset)
 {
+	enum ndr_err_code ret;
 	ndr->relative_base_offset = offset;
-	return ndr_token_store(ndr, &ndr->relative_base_list, p, offset);
+	ret = ndr_token_store(ndr, &ndr->relative_base_list, p, offset);
+	if (ret == NDR_ERR_RANGE) {
+		return ndr_push_error(ndr, ret,
+				      "More than %d NDR tokens stored for relative_base_list",
+				      NDR_TOKEN_MAX_LIST_SIZE);
+	}
+	return ret;
 }
 
 /*
@@ -1501,12 +1569,19 @@ _PUBLIC_ enum ndr_err_code ndr_push_setup_relative_base_offset2(struct ndr_push
 */
 _PUBLIC_ enum ndr_err_code ndr_push_relative_ptr1(struct ndr_push *ndr, const void *p)
 {
+	enum ndr_err_code ret;
 	if (p == NULL) {
 		NDR_CHECK(ndr_push_uint32(ndr, NDR_SCALARS, 0));
 		return NDR_ERR_SUCCESS;
 	}
 	NDR_CHECK(ndr_push_align(ndr, 4));
-	NDR_CHECK(ndr_token_store(ndr, &ndr->relative_list, p, ndr->offset));
+	ret = ndr_token_store(ndr, &ndr->relative_list, p, ndr->offset);
+	if (ret == NDR_ERR_RANGE) {
+		return ndr_push_error(ndr, ret,
+				      "More than %d NDR tokens stored for relative_list",
+				      NDR_TOKEN_MAX_LIST_SIZE);
+	}
+	NDR_CHECK(ret);
 	return ndr_push_uint32(ndr, NDR_SCALARS, 0xFFFFFFFF);
 }
 
@@ -1516,12 +1591,19 @@ _PUBLIC_ enum ndr_err_code ndr_push_relative_ptr1(struct ndr_push *ndr, const vo
 */
 _PUBLIC_ enum ndr_err_code ndr_push_short_relative_ptr1(struct ndr_push *ndr, const void *p)
 {
+	enum ndr_err_code ret;
 	if (p == NULL) {
 		NDR_CHECK(ndr_push_uint16(ndr, NDR_SCALARS, 0));
 		return NDR_ERR_SUCCESS;
 	}
 	NDR_CHECK(ndr_push_align(ndr, 2));
-	NDR_CHECK(ndr_token_store(ndr, &ndr->relative_list, p, ndr->offset));
+	ret = ndr_token_store(ndr, &ndr->relative_list, p, ndr->offset);
+	if (ret == NDR_ERR_RANGE) {
+		return ndr_push_error(ndr, ret,
+				      "More than %d NDR tokens stored for relative_list",
+				      NDR_TOKEN_MAX_LIST_SIZE);
+	}
+	NDR_CHECK(ret);
 	return ndr_push_uint16(ndr, NDR_SCALARS, 0xFFFF);
 }
 /*
@@ -1617,6 +1699,7 @@ _PUBLIC_ enum ndr_err_code ndr_push_short_relative_ptr2(struct ndr_push *ndr, co
 */
 _PUBLIC_ enum ndr_err_code ndr_push_relative_ptr2_start(struct ndr_push *ndr, const void *p)
 {
+	enum ndr_err_code ret;
 	if (p == NULL) {
 		return NDR_ERR_SUCCESS;
 	}
@@ -1655,8 +1738,16 @@ _PUBLIC_ enum ndr_err_code ndr_push_relative_ptr2_start(struct ndr_push *ndr, co
 			      "ndr_push_relative_ptr2_start RELATIVE_REVERSE flag set and relative_end_offset %d",
 			      ndr->relative_end_offset);
 	}
-	NDR_CHECK(ndr_token_store(ndr, &ndr->relative_begin_list, p, ndr->offset));
-	return NDR_ERR_SUCCESS;
+	ret = ndr_token_store(ndr,
+			      &ndr->relative_begin_list,
+			      p,
+			      ndr->offset);
+	if (ret == NDR_ERR_RANGE) {
+		return ndr_push_error(ndr, ret,
+				      "More than %d NDR tokens stored for array_size",
+				      NDR_TOKEN_MAX_LIST_SIZE);
+	}
+	return ret;
 }
 
 /*
@@ -1786,8 +1877,15 @@ _PUBLIC_ void ndr_pull_restore_relative_base_offset(struct ndr_pull *ndr, uint32
 */
 _PUBLIC_ enum ndr_err_code ndr_pull_setup_relative_base_offset1(struct ndr_pull *ndr, const void *p, uint32_t offset)
 {
+	enum ndr_err_code ret;
 	ndr->relative_base_offset = offset;
-	return ndr_token_store(ndr, &ndr->relative_base_list, p, offset);
+	ret = ndr_token_store(ndr, &ndr->relative_base_list, p, offset);
+	if (ret == NDR_ERR_RANGE) {
+		return ndr_pull_error(ndr, ret,
+				      "More than %d NDR tokens stored for relative_base_list",
+				      NDR_TOKEN_MAX_LIST_SIZE);
+	}
+	return ret;
 }
 
 /*
@@ -1805,13 +1903,20 @@ _PUBLIC_ enum ndr_err_code ndr_pull_setup_relative_base_offset2(struct ndr_pull
 */
 _PUBLIC_ enum ndr_err_code ndr_pull_relative_ptr1(struct ndr_pull *ndr, const void *p, uint32_t rel_offset)
 {
+	enum ndr_err_code ret;
 	rel_offset += ndr->relative_base_offset;
 	if (rel_offset > ndr->data_size) {
 		return ndr_pull_error(ndr, NDR_ERR_BUFSIZE,
 				      "ndr_pull_relative_ptr1 rel_offset(%u) > ndr->data_size(%u)",
 				      rel_offset, ndr->data_size);
 	}
-	return ndr_token_store(ndr, &ndr->relative_list, p, rel_offset);
+	ret = ndr_token_store(ndr, &ndr->relative_list, p, rel_offset);
+	if (ret == NDR_ERR_RANGE) {
+		return ndr_pull_error(ndr, ret,
+				      "More than %d NDR tokens stored for relative_list",
+				      NDR_TOKEN_MAX_LIST_SIZE);
+	}
+	return ret;
 }
 
 /*
diff --git a/librpc/ndr/ndr_drsuapi.c b/librpc/ndr/ndr_drsuapi.c
index cd550f1f0ee..b03c1ec57b3 100644
--- a/librpc/ndr/ndr_drsuapi.c
+++ b/librpc/ndr/ndr_drsuapi.c
@@ -420,7 +420,8 @@ enum ndr_err_code ndr_push_drsuapi_DsBindInfo(struct ndr_push *ndr, int ndr_flag
 	ndr->flags = ndr->flags & ~LIBNDR_FLAG_NDR64;
 	NDR_PUSH_CHECK_FLAGS(ndr, ndr_flags);
 	if (ndr_flags & NDR_SCALARS) {
-		uint32_t level = ndr_push_get_switch_value(ndr, r);
+		uint32_t level;
+		NDR_CHECK(ndr_push_steal_switch_value(ndr, r, &level));
 		NDR_CHECK(ndr_push_union_align(ndr, 4));
 		switch (level) {
 			case 24: {
@@ -470,38 +471,18 @@ enum ndr_err_code ndr_push_drsuapi_DsBindInfo(struct ndr_push *ndr, int ndr_flag
 
 		}
 	}
-	if (ndr_flags & NDR_BUFFERS) {
-		uint32_t level = ndr_push_get_switch_value(ndr, r);
-		switch (level) {
-			case 24:
-			break;
-
-			case 28:
-			break;
-
-			case 48:
-			break;
-
-			case 52:
-			break;
-
-			default:
-			break;
-
-		}
-	}
 	ndr->flags = _flags_save;
 	return NDR_ERR_SUCCESS;
 }
 
 enum ndr_err_code ndr_pull_drsuapi_DsBindInfo(struct ndr_pull *ndr, int ndr_flags, union drsuapi_DsBindInfo *r)
 {
-	uint32_t level;
 	uint32_t _flags_save = ndr->flags;
 	ndr->flags = ndr->flags & ~LIBNDR_FLAG_NDR64;
-	level = ndr_pull_get_switch_value(ndr, r);
 	NDR_PULL_CHECK_FLAGS(ndr, ndr_flags);
 	if (ndr_flags & NDR_SCALARS) {
+		uint32_t level;
+		NDR_CHECK(ndr_pull_steal_switch_value(ndr, r, &level));
 		NDR_CHECK(ndr_pull_union_align(ndr, 4));
 		switch (level) {
 			case 24: {
@@ -551,25 +532,6 @@ enum ndr_err_code ndr_pull_drsuapi_DsBindInfo(struct ndr_pull *ndr, int ndr_flag
 
 		}
 	}
-	if (ndr_flags & NDR_BUFFERS) {
-		switch (level) {
-			case 24:
-			break;
-
-			case 28:
-			break;
-
-			case 48:
-			break;
-
-			case 52:
-			break;
-
-			default:
-			break;
-
-		}
-	}
 	ndr->flags = _flags_save;
 	return NDR_ERR_SUCCESS;
 }
@@ -577,7 +539,7 @@ enum ndr_err_code ndr_pull_drsuapi_DsBindInfo(struct ndr_pull *ndr, int ndr_flag
 _PUBLIC_ void ndr_print_drsuapi_DsBindInfo(struct ndr_print *ndr, const char *name, const union drsuapi_DsBindInfo *r)
 {
 	uint32_t level;
-	level = ndr_print_get_switch_value(ndr, r);
+	level = ndr_print_steal_switch_value(ndr, r);
 	ndr_print_union(ndr, name, level, "drsuapi_DsBindInfo");
 	switch (level) {
 		case 24:
diff --git a/librpc/ndr/ndr_negoex.c b/librpc/ndr/ndr_negoex.c
index b5cb5bc8bcf..95adce5a7e3 100644
--- a/librpc/ndr/ndr_negoex.c
+++ b/librpc/ndr/ndr_negoex.c


-- 
Samba Shared Repository



More information about the samba-cvs mailing list