r4110: fixed pidl to allow arrays to have size_is() and length_is() elements

that depend on variables that come after the array in the structure or function.

This has been something that has been problematic for a while, but the
winreg QueryValue problem finally prompted me to fix it properly. We
should now go back and fix up all the ugly workarounds we have used to
avoid this problem in other calls.

Unfortunately the solution is fairly complex, and involves the use of
the internal ndr token lists (similar to the solution for relative
pointers). I wonder if anyone else will be able to follow the logic if
I get run over by a bus :-)
(This used to be commit e839b19ec5581f669f2a7705b1fb80845313251c)

3 files changed, 152 insertions, 65 deletions

diff --git a/source4/build/pidl/parser.pm b/source4/build/pidl/parser.pm
index cec983724d..4461bb77af 100644
--- a/source4/build/pidl/parser.pm
+++ b/source4/build/pidl/parser.pm
@@ -269,6 +269,25 @@ sub ParseArrayPrint($$)
 }
 
 #####################################################################
+# check the size_is and length_is constraints
+sub CheckArraySizes($$)
+{
+	my $e = shift;
+	my $var_prefix = shift;
+
+	if (util::has_property($e, "size_is")) {
+		my $size = find_size_var($e, util::array_size($e), $var_prefix);
+		pidl "\tNDR_CHECK(ndr_check_array_size(ndr, (void*)&$var_prefix$e->{NAME}, $size));\n";
+	}
+
+	if (my $length = util::has_property($e, "length_is")) {
+		$length = find_size_var($e, $length, $var_prefix);
+		pidl "\tNDR_CHECK(ndr_check_array_length(ndr, (void*)&$var_prefix$e->{NAME}, $length));\n";
+	}
+}
+
+
+#####################################################################
 # parse an array - pull side
 sub ParseArrayPull($$$)
 {
@@ -294,47 +313,28 @@ sub ParseArrayPull($$$)
 		}
 
 		# non fixed arrays encode the size just before the array
-		pidl "\t{\n";
-		pidl "\t\tuint32_t _array_size;\n";
-		pidl "\t\tNDR_CHECK(ndr_pull_uint32(ndr, &_array_size));\n";
-		if ($size =~ /r->in/) {
-			pidl "\t\tif (!(ndr->flags & LIBNDR_FLAG_REF_ALLOC) && _array_size != $size) {\n";
-		} else {
-			pidl "\t\tif ($size != _array_size) {\n";
-		}
-		pidl "\t\t\treturn ndr_pull_error(ndr, NDR_ERR_ARRAY_SIZE, \"Bad array size %u should be %u\", _array_size, $size);\n";
-		pidl "\t\t}\n";
-		if ($size =~ /r->in/) {
-			pidl "else { $size = _array_size; }\n";
-		}
-		pidl "\t}\n";
+		pidl "\t\tNDR_CHECK(ndr_pull_array_size(ndr, &$var_prefix$e->{NAME}));\n";
+		$alloc_size = "ndr_get_array_size(ndr, &$var_prefix$e->{NAME})";
 	}
 
 	if ((util::need_alloc($e) && !util::is_fixed_array($e)) ||
 	    ($var_prefix eq "r->in." && util::has_property($e, "ref"))) {
 		if (!util::is_inline_array($e) || $ndr_flags eq "NDR_SCALARS") {
-			pidl "\t\tNDR_ALLOC_N(ndr, $var_prefix$e->{NAME}, MAX(1, $alloc_size));\n";
+			pidl "\t\tNDR_ALLOC_N(ndr, $var_prefix$e->{NAME}, $alloc_size);\n";
 		}
 	}
 
 	if (($var_prefix eq "r->out." && util::has_property($e, "ref"))) {
 		if (!util::is_inline_array($e) || $ndr_flags eq "NDR_SCALARS") {
 			pidl "\tif (ndr->flags & LIBNDR_FLAG_REF_ALLOC) {";
-			pidl "\t\tNDR_ALLOC_N(ndr, $var_prefix$e->{NAME}, MAX(1, $alloc_size));\n";
+			pidl "\t\tNDR_ALLOC_N(ndr, $var_prefix$e->{NAME}, $alloc_size);\n";
 			pidl "\t}\n";
 		}
 	}
 
-	pidl "\t{\n";
-
 	if (my $length = util::has_property($e, "length_is")) {
-		$length = find_size_var($e, $length, $var_prefix);
-		pidl "\t\tuint32_t _offset, _length;\n";
-		pidl "\t\tNDR_CHECK(ndr_pull_uint32(ndr, &_offset));\n";
-		pidl "\t\tNDR_CHECK(ndr_pull_uint32(ndr, &_length));\n";
-		pidl "\t\tif (_offset != 0) return ndr_pull_error(ndr, NDR_ERR_OFFSET, \"Bad array offset 0x%08x\", _offset);\n";
-		pidl "\t\tif (_length > $size || _length != $length) return ndr_pull_error(ndr, NDR_ERR_LENGTH, \"Bad array length 0x%08x > size 0x%08x\", _offset, $size);\n\n";
-		$size = "_length";
+		pidl "\t\tNDR_CHECK(ndr_pull_array_length(ndr, &$var_prefix$e->{NAME}));\n";
+		$size = "ndr_get_array_length(ndr, &$var_prefix$e->{NAME})";
 	}
 
 	if (util::is_scalar_type($e->{TYPE})) {
@@ -342,8 +342,6 @@ sub ParseArrayPull($$$)
 	} else {
 		pidl "\t\tNDR_CHECK(ndr_pull_array(ndr, $ndr_flags, (void **)$var_prefix$e->{NAME}, sizeof($var_prefix$e->{NAME}\[0]), $size, (ndr_pull_flags_fn_t)ndr_pull_$e->{TYPE}));\n";
 	}
-
-	pidl "\t}\n";
 }
 
 
@@ -835,6 +833,10 @@ sub ParseStructPull($)
 		ParseElementPullBuffer($e, "r->", "NDR_BUFFERS");
 	}
 
+	foreach my $e (@{$struct->{ELEMENTS}}) {
+		CheckArraySizes($e, "r->");
+	}
+
 	pidl "\tndr_pull_struct_end(ndr);\n";
 
 	pidl "done:\n";
@@ -844,7 +846,6 @@ sub ParseStructPull($)
 
 #####################################################################
 # calculate size of ndr struct
-
 sub ParseStructNdrSize($)
 {
 	my $t = shift;
@@ -855,7 +856,6 @@ sub ParseStructNdrSize($)
 	pidl "{\n";
 
 	if (util::has_property($t->{DATA}, "flag")) {
-		
 		pidl "\tflags = flags | " . $t->{DATA}->{PROPERTIES}->{flag} . ";\n";	
 	}
 
@@ -1398,7 +1398,7 @@ sub AllocateRefVars($)
 
 	# its an array
 	my $size = find_size_var($e, $asize, "r->out.");
-	pidl "\tNDR_ALLOC_N(ndr, r->out.$e->{NAME}, MAX(1, $size));\n";
+	pidl "\tNDR_ALLOC_N(ndr, r->out.$e->{NAME}, $size);\n";
 	if (util::has_property($e, "in")) {
 		pidl "\tmemcpy(r->out.$e->{NAME},r->in.$e->{NAME},$size * sizeof(*r->in.$e->{NAME}));\n";
 	} else {
@@ -1448,6 +1448,12 @@ sub ParseFunctionPull($)
 		}
 	}
 
+	foreach my $e (@{$fn->{DATA}}) {
+		if (util::has_property($e, "in")) {
+			CheckArraySizes($e, "r->in.");
+		}
+	}
+
 	pidl "\nndr_out:\n";
 	pidl "\tif (!(flags & NDR_OUT)) goto done;\n\n";
 
@@ -1457,6 +1463,12 @@ sub ParseFunctionPull($)
 		}
 	}
 
+	foreach my $e (@{$fn->{DATA}}) {
+		if (util::has_property($e, "out")) {
+			CheckArraySizes($e, "r->out.");
+		}
+	}
+
 	if ($fn->{RETURN_TYPE} && $fn->{RETURN_TYPE} ne "void") {
 		pidl "\tNDR_CHECK(ndr_pull_$fn->{RETURN_TYPE}(ndr, &r->out.result));\n";
 	}
diff --git a/source4/librpc/ndr/libndr.h b/source4/librpc/ndr/libndr.h
index fe817bfdcd..bb28c3a23c 100644
--- a/source4/librpc/ndr/libndr.h
+++ b/source4/librpc/ndr/libndr.h
@@ -47,6 +47,8 @@ struct ndr_pull {
 	uint32_t offset;
 
 	struct ndr_token_list *relative_list;
+	struct ndr_token_list *array_size_list;
+	struct ndr_token_list *array_length_list;
 
 	/* this is used to ensure we generate unique reference IDs
 	   between request and reply */
@@ -149,7 +151,8 @@ enum ndr_err_code {
 	NDR_ERR_VALIDATE,
 	NDR_ERR_BUFSIZE,
 	NDR_ERR_ALLOC,
-	NDR_ERR_RANGE
+	NDR_ERR_RANGE,
+	NDR_ERR_TOKEN
 };
 
 /*
@@ -236,30 +239,20 @@ enum ndr_err_code {
 
 
 #define NDR_ALLOC_N_SIZE(ndr, s, n, elsize) do { \
-				if ((n) == 0) { \
-					(s) = NULL; \
-				} else { \
-					(s) = talloc_array(ndr, elsize, n, __location__); \
-                               		if (!(s)) return ndr_pull_error(ndr, \
-									NDR_ERR_ALLOC, \
-									"Alloc %u * %u failed\n", \
-									n, elsize); \
-				} \
-                           } while (0)
+	(s) = talloc_array(ndr, elsize, n, __location__); \
+	if (!(s)) return ndr_pull_error(ndr, NDR_ERR_ALLOC, "Alloc %u * %u failed\n", n, elsize); \
+} while (0)
 
 #define NDR_ALLOC_N(ndr, s, n) NDR_ALLOC_N_SIZE(ndr, s, n, sizeof(*(s)))
 
 
 #define NDR_PUSH_ALLOC_SIZE(ndr, s, size) do { \
-	                       (s) = talloc(ndr, size); \
-                               if ((size) && !(s)) return ndr_push_error(ndr, NDR_ERR_ALLOC, \
-							       "push alloc %u failed\n",\
-							       size); \
-                           } while (0)
+       (s) = talloc(ndr, size); \
+       if (!(s)) return ndr_push_error(ndr, NDR_ERR_ALLOC, "push alloc %u failed\n", size); \
+} while (0)
 
 #define NDR_PUSH_ALLOC(ndr, s) NDR_PUSH_ALLOC_SIZE(ndr, s, sizeof(*(s)))
 
-
 /* these are used when generic fn pointers are needed for ndr push/pull fns */
 typedef NTSTATUS (*ndr_push_fn_t)(struct ndr_push *, void *);
 typedef NTSTATUS (*ndr_pull_fn_t)(struct ndr_pull *, void *);
diff --git a/source4/librpc/ndr/ndr.c b/source4/librpc/ndr/ndr.c
index 857e171224..7b55f4efb7 100644
--- a/source4/librpc/ndr/ndr.c
+++ b/source4/librpc/ndr/ndr.c
@@ -48,15 +48,11 @@ struct ndr_pull *ndr_pull_init_blob(const DATA_BLOB *blob, TALLOC_CTX *mem_ctx)
 {
 	struct ndr_pull *ndr;
 
-	ndr = talloc_p(mem_ctx, struct ndr_pull);
+	ndr = talloc_zero_p(mem_ctx, struct ndr_pull);
 	if (!ndr) return NULL;
 
-	ndr->flags = 0;
 	ndr->data = blob->data;
 	ndr->data_size = blob->length;
-	ndr->offset = 0;
-	ndr->ptr_count = 0;
-	ndr->relative_list = NULL;
 
 	return ndr;
 }
@@ -125,7 +121,7 @@ struct ndr_push *ndr_push_init_ctx(TALLOC_CTX *mem_ctx)
 {
 	struct ndr_push *ndr;
 
-	ndr = talloc_p(mem_ctx, struct ndr_push);
+	ndr = talloc_zero_p(mem_ctx, struct ndr_push);
 	if (!ndr) {
 		return NULL;
 	}
@@ -136,9 +132,6 @@ struct ndr_push *ndr_push_init_ctx(TALLOC_CTX *mem_ctx)
 	if (!ndr->data) {
 		return NULL;
 	}
-	ndr->offset = 0;
-	ndr->ptr_count = 0;
-	ndr->relative_list = NULL;
 
 	return ndr;
 }
@@ -381,8 +374,12 @@ static NTSTATUS ndr_map_error(enum ndr_err_code err)
 	switch (err) {
 	case NDR_ERR_BUFSIZE:
 		return NT_STATUS_BUFFER_TOO_SMALL;
+	case NDR_ERR_TOKEN:
+		return NT_STATUS_INTERNAL_ERROR;
 	case NDR_ERR_ALLOC:
 		return NT_STATUS_NO_MEMORY;
+	case NDR_ERR_ARRAY_SIZE:
+		return NT_STATUS_ARRAY_BOUNDS_EXCEEDED;
 	default:
 		break;
 	}
@@ -655,18 +652,109 @@ static NTSTATUS ndr_token_store(TALLOC_CTX *mem_ctx,
 /*
   retrieve a token from a ndr context
 */
-static uint32_t ndr_token_retrieve(struct ndr_token_list **list, const void *key)
+static NTSTATUS ndr_token_retrieve(struct ndr_token_list **list, const void *key, uint32_t *v)
 {
 	struct ndr_token_list *tok;
 	for (tok=*list;tok;tok=tok->next) {
 		if (tok->key == key) {
 			DLIST_REMOVE((*list), tok);
+			*v = tok->value;
+			return NT_STATUS_OK;
+		}
+	}
+	return ndr_map_error(NDR_ERR_TOKEN);
+}
+
+/*
+  peek at but don't removed a token from a ndr context
+*/
+static uint32_t ndr_token_peek(struct ndr_token_list **list, const void *key)
+{
+	struct ndr_token_list *tok;
+	for (tok=*list;tok;tok=tok->next) {
+		if (tok->key == key) {
 			return tok->value;
 		}
 	}
 	return 0;
 }
 
+/*
+  pull an array size field and add it to the array_size_list token list
+*/
+NTSTATUS ndr_pull_array_size(struct ndr_pull *ndr, const void *p)
+{
+	uint32_t size;
+	NDR_CHECK(ndr_pull_uint32(ndr, &size));
+	return ndr_token_store(ndr, &ndr->array_size_list, p, size);
+}
+
+/*
+  get the stored array size field
+*/
+uint32_t ndr_get_array_size(struct ndr_pull *ndr, const void *p)
+{
+	return ndr_token_peek(&ndr->array_size_list, p);
+}
+
+/*
+  check the stored array size field
+*/
+NTSTATUS ndr_check_array_size(struct ndr_pull *ndr, const void **p, uint32_t size)
+{
+	uint32 stored;
+	if (*p == NULL) {
+		return NT_STATUS_OK;
+	}
+	NDR_CHECK(ndr_token_retrieve(&ndr->array_size_list, 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 NT_STATUS_OK;
+}
+
+/*
+  pull an array length field and add it to the array_length_list token list
+*/
+NTSTATUS ndr_pull_array_length(struct ndr_pull *ndr, const void *p)
+{
+	uint32_t length, offset;
+	NDR_CHECK(ndr_pull_uint32(ndr, &offset));
+	if (offset != 0) {
+		return ndr_pull_error(ndr, NDR_ERR_ARRAY_SIZE, 
+				      "non-zero array offset %u\n", offset);
+	}
+	NDR_CHECK(ndr_pull_uint32(ndr, &length));
+	return ndr_token_store(ndr, &ndr->array_length_list, p, length);
+}
+
+/*
+  get the stored array length field
+*/
+uint32_t ndr_get_array_length(struct ndr_pull *ndr, const void *p)
+{
+	return ndr_token_peek(&ndr->array_length_list, p);
+}
+
+/*
+  check the stored array length field
+*/
+NTSTATUS ndr_check_array_length(struct ndr_pull *ndr, void *p, uint32_t length)
+{
+	uint32_t stored;
+	if (*(void **)p == NULL) {
+		return NT_STATUS_OK;
+	}
+	NDR_CHECK(ndr_token_retrieve(&ndr->array_length_list, p, &stored));
+	if (stored != length) {
+		return ndr_pull_error(ndr, NDR_ERR_ARRAY_SIZE, 
+				      "Bad array length - got %u expected %u\n",
+				      stored, length);
+	}
+	return NT_STATUS_OK;
+}
 
 /*
   pull a relative object - stage1
@@ -689,10 +777,7 @@ NTSTATUS ndr_pull_relative1(struct ndr_pull *ndr, const void *p, uint32_t rel_of
 NTSTATUS ndr_pull_relative2(struct ndr_pull *ndr, const void *p)
 {
 	uint32_t rel_offset;
-	rel_offset = ndr_token_retrieve(&ndr->relative_list, p);
-	if (rel_offset == 0) {
-		return NT_STATUS_INTERNAL_ERROR;
-	}
+	NDR_CHECK(ndr_token_retrieve(&ndr->relative_list, p, &rel_offset));
 	return ndr_pull_set_offset(ndr, rel_offset);
 }
 
@@ -723,10 +808,7 @@ NTSTATUS ndr_push_relative2(struct ndr_push *ndr, const void *p)
 	}
 	NDR_CHECK(ndr_push_align(ndr, 4));
 	ndr_push_save(ndr, &save);
-	ndr->offset = ndr_token_retrieve(&ndr->relative_list, p);
-	if (ndr->offset == 0) {
-		return NT_STATUS_INTERNAL_ERROR;
-	}
+	NDR_CHECK(ndr_token_retrieve(&ndr->relative_list, p, &ndr->offset));
 	if (ndr->flags & LIBNDR_FLAG_RELATIVE_CURRENT) {
 		NDR_CHECK(ndr_push_uint32(ndr, save.offset - ndr->offset));
 	} else {