1
0
mirror of https://github.com/samba-team/samba.git synced 2025-01-08 21:18:16 +03:00

pidl: Avoid leaving array_size NDR tokens around

In many cases these can and should be consumed as soon as
they are used.

This is not a complete fix, we don't clean up the array_size
token after using it split between an NDR_SCALARS and
an NDR_BUFFERS pass, but it is much better than it was
and helps the winbind case with a large number of groups
(eg 100,000) as otherwise we hit the 65535 NDR token limit.

(This is an arbitary Samba-only limit to avoid DoS conditions)

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14710

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
This commit is contained in:
Andrew Bartlett 2021-05-22 18:40:13 +12:00 committed by Douglas Bagnall
parent a7d4f93cfd
commit 3bc680c1e3
7 changed files with 51 additions and 9 deletions

View File

@ -20,6 +20,7 @@ ndr_check_array_size: enum ndr_err_code (struct ndr_pull *, const void *, uint32
ndr_check_padding: void (struct ndr_pull *, size_t)
ndr_check_pipe_chunk_trailer: enum ndr_err_code (struct ndr_pull *, int, uint32_t)
ndr_check_steal_array_length: enum ndr_err_code (struct ndr_pull *, const void *, uint32_t)
ndr_check_steal_array_size: enum ndr_err_code (struct ndr_pull *, const void *, uint32_t)
ndr_check_string_terminator: enum ndr_err_code (struct ndr_pull *, uint32_t, uint32_t)
ndr_get_array_length: enum ndr_err_code (struct ndr_pull *, const void *, uint32_t *)
ndr_get_array_size: enum ndr_err_code (struct ndr_pull *, const void *, uint32_t *)
@ -250,6 +251,7 @@ ndr_size_struct: size_t (const void *, int, ndr_push_flags_fn_t)
ndr_size_union: size_t (const void *, int, uint32_t, ndr_push_flags_fn_t)
ndr_size_winreg_Data_GPO: size_t (const union winreg_Data_GPO *, uint32_t, int)
ndr_steal_array_length: enum ndr_err_code (struct ndr_pull *, const void *, uint32_t *)
ndr_steal_array_size: enum ndr_err_code (struct ndr_pull *, const void *, uint32_t *)
ndr_string_array_size: size_t (struct ndr_push *, const char *)
ndr_string_length: uint32_t (const void *, uint32_t)
ndr_syntax_id_buf_string: char *(const struct ndr_syntax_id *, struct ndr_syntax_id_buf *)

View File

@ -656,7 +656,9 @@ enum ndr_err_code ndr_token_retrieve(struct ndr_token_list *list, const void *ke
enum ndr_err_code ndr_token_peek(struct ndr_token_list *list, const void *key, uint32_t *v);
enum ndr_err_code ndr_pull_array_size(struct ndr_pull *ndr, const void *p);
enum ndr_err_code ndr_get_array_size(struct ndr_pull *ndr, const void *p, uint32_t *size);
enum ndr_err_code ndr_steal_array_size(struct ndr_pull *ndr, const void *p, uint32_t *size);
enum ndr_err_code ndr_check_array_size(struct ndr_pull *ndr, const void *p, uint32_t size);
enum ndr_err_code ndr_check_steal_array_size(struct ndr_pull *ndr, const void *p, uint32_t size);
enum ndr_err_code ndr_pull_array_length(struct ndr_pull *ndr, const void *p);
enum ndr_err_code ndr_get_array_length(struct ndr_pull *ndr, const void *p, uint32_t *length);
enum ndr_err_code ndr_steal_array_length(struct ndr_pull *ndr, const void *p, uint32_t *length);

View File

@ -1098,8 +1098,34 @@ _PUBLIC_ enum ndr_err_code ndr_get_array_size(struct ndr_pull *ndr, const void *
}
/*
check the stored array size field
get and remove from the stored list the stored array size field
*/
_PUBLIC_ enum ndr_err_code ndr_steal_array_size(struct ndr_pull *ndr, const void *p, uint32_t *size)
{
return ndr_token_retrieve(&ndr->array_size_list, p, size);
}
/*
* check the stored array size field and remove from the stored list
* (the array_size NDR token list). We try to remove when possible to
* avoid the list growing towards the bounds check
*/
_PUBLIC_ enum ndr_err_code ndr_check_steal_array_size(struct ndr_pull *ndr, const void *p, uint32_t size)
{
uint32_t stored;
NDR_CHECK(ndr_steal_array_size(ndr, p, &stored));
if (stored != size) {
return ndr_pull_error(ndr, NDR_ERR_ARRAY_SIZE,
"Bad array size - got %u expected %u\n",
stored, size);
}
return NDR_ERR_SUCCESS;
}
/*
* check the stored array size field (leaving it on the array_size
* token list)
*/
_PUBLIC_ enum ndr_err_code ndr_check_array_size(struct ndr_pull *ndr, const void *p, uint32_t size)
{
uint32_t stored;

View File

@ -99,7 +99,7 @@ enum ndr_err_code ndr_pull_negoex_BYTE_VECTOR(struct ndr_pull *ndr, int ndr_flag
}
#if 0
if (r->blob.data) {
NDR_CHECK(ndr_check_array_size(ndr, (void*)&r->blob.data, r->blob.length));
NDR_CHECK(ndr_check_steal_array_size(ndr, (void*)&r->blob.data, r->blob.length));
}
#endif
}
@ -179,7 +179,7 @@ enum ndr_err_code ndr_pull_negoex_AUTH_SCHEME_VECTOR(struct ndr_pull *ndr, int n
}
#if 0
if (r->array) {
NDR_CHECK(ndr_check_array_size(ndr, (void*)&r->array, r->count));
NDR_CHECK(ndr_check_steal_array_size(ndr, (void*)&r->array, r->count));
}
#endif
}
@ -265,7 +265,7 @@ enum ndr_err_code ndr_pull_negoex_EXTENSION_VECTOR(struct ndr_pull *ndr, int ndr
}
#if 0
if (r->array) {
NDR_CHECK(ndr_check_array_size(ndr, (void*)&r->array, r->count));
NDR_CHECK(ndr_check_steal_array_size(ndr, (void*)&r->array, r->count));
}
#endif
}
@ -351,7 +351,7 @@ enum ndr_err_code ndr_pull_negoex_ALERT_VECTOR(struct ndr_pull *ndr, int ndr_fla
}
#if 0
if (r->array) {
NDR_CHECK(ndr_check_array_size(ndr, (void*)&r->array, r->count));
NDR_CHECK(ndr_check_steal_array_size(ndr, (void*)&r->array, r->count));
}
#endif
}

View File

@ -1227,7 +1227,7 @@ _PUBLIC_ enum ndr_err_code ndr_pull_spoolss_DriverInfo101(struct ndr_pull *ndr,
ndr->flags = _flags_save_string;
}
if (r->file_info) {
NDR_CHECK(ndr_check_array_size(ndr, (void*)&r->file_info, r->file_count));
NDR_CHECK(ndr_check_steal_array_size(ndr, (void*)&r->file_info, r->file_count));
}
}
return NDR_ERR_SUCCESS;

View File

@ -342,11 +342,17 @@ sub ParseArrayPullGetSize($$$$$$)
my $array_size = "size_$e->{NAME}_$l->{LEVEL_INDEX}";
if ($l->{IS_CONFORMANT}) {
if ($l->{IS_CONFORMANT} and (defined($l->{SIZE_IS}) or not $l->{IS_ZERO_TERMINATED})) {
$self->pidl("NDR_CHECK(ndr_get_array_size($ndr, (void*)" . get_pointer_to($var_name) . ", &$array_size));");
} elsif ($l->{IS_CONFORMANT}) {
# This will be the last use of the array_size token
$self->pidl("NDR_CHECK(ndr_steal_array_size($ndr, (void*)" . get_pointer_to($var_name) . ", &$array_size));");
} elsif ($l->{IS_ZERO_TERMINATED} and $l->{SIZE_IS} == 0 and $l->{LENGTH_IS} == 0) { # Noheader arrays
$size = "ndr_get_string_size($ndr, sizeof(*$var_name))";
$self->pidl("$array_size = $size;");
} else {
$size = ParseExprExt($l->{SIZE_IS}, $env, $e->{ORIGINAL},
check_null_pointer($e, $env, sub { $self->pidl(shift); },
@ -443,7 +449,14 @@ sub ParseArrayPullHeader($$$$$$)
check_null_pointer($e, $env, sub { $self->defer(shift); },
"return ndr_pull_error($ndr, NDR_ERR_INVALID_POINTER, \"NULL Pointer for size_is()\");"),
check_fully_dereferenced($e, $env));
$self->defer("NDR_CHECK(ndr_check_array_size($ndr, (void*)" . get_pointer_to($var_name) . ", $size));");
if (ContainsDeferred($e, $l)) {
# We will be needing the array_size token in
# the NDR_BUFFERS call, so don't steal it now
$self->defer("NDR_CHECK(ndr_check_array_size($ndr, (void*)" . get_pointer_to($var_name) . ", $size));");
} else {
# This will be deferred until after the last ndr_get_array_size()
$self->defer("NDR_CHECK(ndr_check_steal_array_size($ndr, (void*)" . get_pointer_to($var_name) . ", $size));");
}
$self->defer_deindent;
$self->defer("}");
}

View File

@ -1 +0,0 @@
^samba.tests.ndr.samba.tests.ndr.NdrTestCase.test_wbint_max_token_Principals