[SCM] Samba Shared Repository - branch v3-5-test updated

Karolin Seeger kseeger at samba.org
Thu Mar 18 04:31:44 MDT 2010


The branch, v3-5-test has been updated
       via  5543629... s3-winreg: Fix _winreg_QueryValue crash bugs and implement windows behavior.
       via  6740503... s3-winreg: add some debug statements to _winreg_QueryValue().
       via  bd3a850... s3: re-run make samba3-idl.
       via  f9a5264... winreg: fix winreg_QueryValue IDL.
      from  e74ced9... s3-printing: Fix "printer admin" functionality.

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-5-test


- Log -----------------------------------------------------------------
commit 55436299da49d995a2d9b3d7b702122ebb2ce156
Author: Günther Deschner <gd at samba.org>
Date:   Thu Mar 11 12:21:08 2010 +0100

    s3-winreg: Fix _winreg_QueryValue crash bugs and implement windows behavior.
    
    Found by RPC-WINREG smbtorture test.
    
    Guenther
    (cherry picked from commit cddc542ba5f30316ff4ee8fa591a54808b7be4c8)
    
    The last 4 patches address bug #7258 (NULL pointer derref crash in
    _winreg_QueryValue).

commit 6740503b0bace52d09d2a306f8da8b168c2e7ab0
Author: Günther Deschner <gd at samba.org>
Date:   Wed Mar 10 14:17:23 2010 +0100

    s3-winreg: add some debug statements to _winreg_QueryValue().
    
    Guenther
    (cherry picked from commit c5ba525748fdab6b182e35673983719b7c235127)

commit bd3a8505a90eac76b61b354dfc9565c69c164e95
Author: Günther Deschner <gd at samba.org>
Date:   Wed Mar 17 12:06:39 2010 +0100

    s3: re-run make samba3-idl.
    
    Guenther

commit f9a52643a5665efd5b11733179e574e3d0282e8b
Author: Günther Deschner <gd at samba.org>
Date:   Fri Mar 5 21:56:50 2010 +0100

    winreg: fix winreg_QueryValue IDL.
    
    Note that before this change pidl generated code that just dereferenced size_is
    and length_is values from unique pointers without checking whether these
    pointers were actually NULL.
    
    With this change, pidl now throws a warning like:
    
    	warning: Got pointer for `data_size', expected fully derefenced variable
    
    which is not correct, probably because pidl does not evaluate the C expression.
    
    Guenther
    (cherry picked from commit f258e98e177f0f75bab99654b9f32b10bb7ce37f)

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

Summary of changes:
 librpc/gen_ndr/cli_winreg.c        |    8 ++++----
 librpc/gen_ndr/cli_winreg.h        |    4 ++--
 librpc/gen_ndr/ndr_winreg.c        |   24 ++++++++++++------------
 librpc/gen_ndr/winreg.h            |    4 ++--
 librpc/idl/winreg.idl              |    2 +-
 source3/rpc_server/srv_winreg_nt.c |   27 ++++++++++++++-------------
 6 files changed, 35 insertions(+), 34 deletions(-)


Changeset truncated at 500 lines:

diff --git a/librpc/gen_ndr/cli_winreg.c b/librpc/gen_ndr/cli_winreg.c
index 4264542..1c37f51 100644
--- a/librpc/gen_ndr/cli_winreg.c
+++ b/librpc/gen_ndr/cli_winreg.c
@@ -2743,7 +2743,7 @@ struct tevent_req *rpccli_winreg_QueryValue_send(TALLOC_CTX *mem_ctx,
 						 struct policy_handle *_handle /* [in] [ref] */,
 						 struct winreg_String *_value_name /* [in] [ref] */,
 						 enum winreg_Type *_type /* [in,out] [unique] */,
-						 uint8_t *_data /* [in,out] [unique,length_is(*data_length),size_is(*data_size)] */,
+						 uint8_t *_data /* [in,out] [unique,range(0,0x4000000),length_is(data_length?*data_length:0),size_is(data_size?*data_size:0)] */,
 						 uint32_t *_data_size /* [in,out] [unique] */,
 						 uint32_t *_data_length /* [in,out] [unique] */)
 {
@@ -2823,7 +2823,7 @@ static void rpccli_winreg_QueryValue_done(struct tevent_req *subreq)
 		*state->orig.out.type = *state->tmp.out.type;
 	}
 	if (state->orig.out.data && state->tmp.out.data) {
-		memcpy(state->orig.out.data, state->tmp.out.data, (*state->tmp.in.data_size) * sizeof(*state->orig.out.data));
+		memcpy(state->orig.out.data, state->tmp.out.data, (state->tmp.in.data_size?*state->tmp.in.data_size:0) * sizeof(*state->orig.out.data));
 	}
 	if (state->orig.out.data_size && state->tmp.out.data_size) {
 		*state->orig.out.data_size = *state->tmp.out.data_size;
@@ -2869,7 +2869,7 @@ NTSTATUS rpccli_winreg_QueryValue(struct rpc_pipe_client *cli,
 				  struct policy_handle *handle /* [in] [ref] */,
 				  struct winreg_String *value_name /* [in] [ref] */,
 				  enum winreg_Type *type /* [in,out] [unique] */,
-				  uint8_t *data /* [in,out] [unique,length_is(*data_length),size_is(*data_size)] */,
+				  uint8_t *data /* [in,out] [unique,range(0,0x4000000),length_is(data_length?*data_length:0),size_is(data_size?*data_size:0)] */,
 				  uint32_t *data_size /* [in,out] [unique] */,
 				  uint32_t *data_length /* [in,out] [unique] */,
 				  WERROR *werror)
@@ -2904,7 +2904,7 @@ NTSTATUS rpccli_winreg_QueryValue(struct rpc_pipe_client *cli,
 		*type = *r.out.type;
 	}
 	if (data && r.out.data) {
-		memcpy(data, r.out.data, (*r.in.data_size) * sizeof(*data));
+		memcpy(data, r.out.data, (r.in.data_size?*r.in.data_size:0) * sizeof(*data));
 	}
 	if (data_size && r.out.data_size) {
 		*data_size = *r.out.data_size;
diff --git a/librpc/gen_ndr/cli_winreg.h b/librpc/gen_ndr/cli_winreg.h
index 2d7ac0c..bbd9e9a 100644
--- a/librpc/gen_ndr/cli_winreg.h
+++ b/librpc/gen_ndr/cli_winreg.h
@@ -298,7 +298,7 @@ struct tevent_req *rpccli_winreg_QueryValue_send(TALLOC_CTX *mem_ctx,
 						 struct policy_handle *_handle /* [in] [ref] */,
 						 struct winreg_String *_value_name /* [in] [ref] */,
 						 enum winreg_Type *_type /* [in,out] [unique] */,
-						 uint8_t *_data /* [in,out] [unique,length_is(*data_length),size_is(*data_size)] */,
+						 uint8_t *_data /* [in,out] [unique,range(0,0x4000000),length_is(data_length?*data_length:0),size_is(data_size?*data_size:0)] */,
 						 uint32_t *_data_size /* [in,out] [unique] */,
 						 uint32_t *_data_length /* [in,out] [unique] */);
 NTSTATUS rpccli_winreg_QueryValue_recv(struct tevent_req *req,
@@ -309,7 +309,7 @@ NTSTATUS rpccli_winreg_QueryValue(struct rpc_pipe_client *cli,
 				  struct policy_handle *handle /* [in] [ref] */,
 				  struct winreg_String *value_name /* [in] [ref] */,
 				  enum winreg_Type *type /* [in,out] [unique] */,
-				  uint8_t *data /* [in,out] [unique,length_is(*data_length),size_is(*data_size)] */,
+				  uint8_t *data /* [in,out] [unique,range(0,0x4000000),length_is(data_length?*data_length:0),size_is(data_size?*data_size:0)] */,
 				  uint32_t *data_size /* [in,out] [unique] */,
 				  uint32_t *data_length /* [in,out] [unique] */,
 				  WERROR *werror);
diff --git a/librpc/gen_ndr/ndr_winreg.c b/librpc/gen_ndr/ndr_winreg.c
index b2147ef..6f432d3 100644
--- a/librpc/gen_ndr/ndr_winreg.c
+++ b/librpc/gen_ndr/ndr_winreg.c
@@ -2484,10 +2484,10 @@ _PUBLIC_ enum ndr_err_code ndr_push_winreg_QueryValue(struct ndr_push *ndr, int
 		}
 		NDR_CHECK(ndr_push_unique_ptr(ndr, r->in.data));
 		if (r->in.data) {
-			NDR_CHECK(ndr_push_uint3264(ndr, NDR_SCALARS, *r->in.data_size));
+			NDR_CHECK(ndr_push_uint3264(ndr, NDR_SCALARS, r->in.data_size?*r->in.data_size:0));
 			NDR_CHECK(ndr_push_uint3264(ndr, NDR_SCALARS, 0));
-			NDR_CHECK(ndr_push_uint3264(ndr, NDR_SCALARS, *r->in.data_length));
-			NDR_CHECK(ndr_push_array_uint8(ndr, NDR_SCALARS, r->in.data, *r->in.data_length));
+			NDR_CHECK(ndr_push_uint3264(ndr, NDR_SCALARS, r->in.data_length?*r->in.data_length:0));
+			NDR_CHECK(ndr_push_array_uint8(ndr, NDR_SCALARS, r->in.data, r->in.data_length?*r->in.data_length:0));
 		}
 		NDR_CHECK(ndr_push_unique_ptr(ndr, r->in.data_size));
 		if (r->in.data_size) {
@@ -2505,10 +2505,10 @@ _PUBLIC_ enum ndr_err_code ndr_push_winreg_QueryValue(struct ndr_push *ndr, int
 		}
 		NDR_CHECK(ndr_push_unique_ptr(ndr, r->out.data));
 		if (r->out.data) {
-			NDR_CHECK(ndr_push_uint3264(ndr, NDR_SCALARS, *r->out.data_size));
+			NDR_CHECK(ndr_push_uint3264(ndr, NDR_SCALARS, r->out.data_size?*r->out.data_size:0));
 			NDR_CHECK(ndr_push_uint3264(ndr, NDR_SCALARS, 0));
-			NDR_CHECK(ndr_push_uint3264(ndr, NDR_SCALARS, *r->out.data_length));
-			NDR_CHECK(ndr_push_array_uint8(ndr, NDR_SCALARS, r->out.data, *r->out.data_length));
+			NDR_CHECK(ndr_push_uint3264(ndr, NDR_SCALARS, r->out.data_length?*r->out.data_length:0));
+			NDR_CHECK(ndr_push_array_uint8(ndr, NDR_SCALARS, r->out.data, r->out.data_length?*r->out.data_length:0));
 		}
 		NDR_CHECK(ndr_push_unique_ptr(ndr, r->out.data_size));
 		if (r->out.data_size) {
@@ -2608,11 +2608,11 @@ _PUBLIC_ enum ndr_err_code ndr_pull_winreg_QueryValue(struct ndr_pull *ndr, int
 		}
 		if (r->in.data) {
 			if (r->in.data_size == NULL) return ndr_pull_error(ndr, NDR_ERR_INVALID_POINTER, "NULL Pointer for size_is()");
-			NDR_CHECK(ndr_check_array_size(ndr, (void*)&r->in.data, *r->in.data_size));
+			NDR_CHECK(ndr_check_array_size(ndr, (void*)&r->in.data, r->in.data_size?*r->in.data_size:0));
 		}
 		if (r->in.data) {
 			if (r->in.data_length == NULL) return ndr_pull_error(ndr, NDR_ERR_INVALID_POINTER, "NULL Pointer for length_is()");
-			NDR_CHECK(ndr_check_array_length(ndr, (void*)&r->in.data, *r->in.data_length));
+			NDR_CHECK(ndr_check_array_length(ndr, (void*)&r->in.data, r->in.data_length?*r->in.data_length:0));
 		}
 	}
 	if (flags & NDR_OUT) {
@@ -2673,11 +2673,11 @@ _PUBLIC_ enum ndr_err_code ndr_pull_winreg_QueryValue(struct ndr_pull *ndr, int
 		NDR_CHECK(ndr_pull_WERROR(ndr, NDR_SCALARS, &r->out.result));
 		if (r->out.data) {
 			if (r->out.data_size == NULL) return ndr_pull_error(ndr, NDR_ERR_INVALID_POINTER, "NULL Pointer for size_is()");
-			NDR_CHECK(ndr_check_array_size(ndr, (void*)&r->out.data, *r->out.data_size));
+			NDR_CHECK(ndr_check_array_size(ndr, (void*)&r->out.data, r->out.data_size?*r->out.data_size:0));
 		}
 		if (r->out.data) {
 			if (r->out.data_length == NULL) return ndr_pull_error(ndr, NDR_ERR_INVALID_POINTER, "NULL Pointer for length_is()");
-			NDR_CHECK(ndr_check_array_length(ndr, (void*)&r->out.data, *r->out.data_length));
+			NDR_CHECK(ndr_check_array_length(ndr, (void*)&r->out.data, r->out.data_length?*r->out.data_length:0));
 		}
 	}
 	return NDR_ERR_SUCCESS;
@@ -2711,7 +2711,7 @@ _PUBLIC_ void ndr_print_winreg_QueryValue(struct ndr_print *ndr, const char *nam
 		ndr->depth++;
 		if (r->in.data) {
 			if (r->in.data_length == NULL) return;
-			ndr_print_array_uint8(ndr, "data", r->in.data, *r->in.data_length);
+			ndr_print_array_uint8(ndr, "data", r->in.data, r->in.data_length?*r->in.data_length:0);
 		}
 		ndr->depth--;
 		ndr_print_ptr(ndr, "data_size", r->in.data_size);
@@ -2741,7 +2741,7 @@ _PUBLIC_ void ndr_print_winreg_QueryValue(struct ndr_print *ndr, const char *nam
 		ndr->depth++;
 		if (r->out.data) {
 			if (r->out.data_length == NULL) return;
-			ndr_print_array_uint8(ndr, "data", r->out.data, *r->out.data_length);
+			ndr_print_array_uint8(ndr, "data", r->out.data, r->out.data_length?*r->out.data_length:0);
 		}
 		ndr->depth--;
 		ndr_print_ptr(ndr, "data_size", r->out.data_size);
diff --git a/librpc/gen_ndr/winreg.h b/librpc/gen_ndr/winreg.h
index 7116810..53d9a35 100644
--- a/librpc/gen_ndr/winreg.h
+++ b/librpc/gen_ndr/winreg.h
@@ -362,14 +362,14 @@ struct winreg_QueryValue {
 		struct policy_handle *handle;/* [ref] */
 		struct winreg_String *value_name;/* [ref] */
 		enum winreg_Type *type;/* [unique] */
-		uint8_t *data;/* [unique,length_is(*data_length),size_is(*data_size)] */
+		uint8_t *data;/* [unique,range(0,0x4000000),length_is(data_length?*data_length:0),size_is(data_size?*data_size:0)] */
 		uint32_t *data_size;/* [unique] */
 		uint32_t *data_length;/* [unique] */
 	} in;
 
 	struct {
 		enum winreg_Type *type;/* [unique] */
-		uint8_t *data;/* [unique,length_is(*data_length),size_is(*data_size)] */
+		uint8_t *data;/* [unique,range(0,0x4000000),length_is(data_length?*data_length:0),size_is(data_size?*data_size:0)] */
 		uint32_t *data_size;/* [unique] */
 		uint32_t *data_length;/* [unique] */
 		WERROR result;
diff --git a/librpc/idl/winreg.idl b/librpc/idl/winreg.idl
index f1f4dfb..7020691 100644
--- a/librpc/idl/winreg.idl
+++ b/librpc/idl/winreg.idl
@@ -256,7 +256,7 @@ import "lsa.idl", "security.idl", "misc.idl";
 		[in,ref] policy_handle *handle,
 		[in,ref] winreg_String *value_name,
 		[in,out,unique] winreg_Type *type,
-		[in,out,unique,size_is(*data_size),length_is(*data_length)] uint8 *data,
+		[in,out,unique,size_is(data_size ? *data_size : 0),length_is(data_length ? *data_length : 0),range(0,0x4000000)] uint8 *data,
 		[in,out,unique] uint32 *data_size,
 		[in,out,unique] uint32 *data_length
 	);
diff --git a/source3/rpc_server/srv_winreg_nt.c b/source3/rpc_server/srv_winreg_nt.c
index dcfe2b9..5912322 100644
--- a/source3/rpc_server/srv_winreg_nt.c
+++ b/source3/rpc_server/srv_winreg_nt.c
@@ -230,14 +230,12 @@ WERROR _winreg_QueryValue(pipes_struct *p, struct winreg_QueryValue *r)
 	if ( !regkey )
 		return WERR_BADFID;
 
-	if ((r->out.data_length == NULL) || (r->out.type == NULL)) {
+	if ((r->out.data_length == NULL) || (r->out.type == NULL) || (r->out.data_size == NULL)) {
 		return WERR_INVALID_PARAM;
 	}
 
-	*r->out.data_length = *r->out.type = REG_NONE;
-
-	DEBUG(7,("_reg_info: policy key name = [%s]\n", regkey->key->name));
-	DEBUG(7,("_reg_info: policy key type = [%08x]\n", regkey->key->type));
+	DEBUG(7,("_winreg_QueryValue: policy key name = [%s]\n", regkey->key->name));
+	DEBUG(7,("_winreg_QueryValue: policy key type = [%08x]\n", regkey->key->type));
 
 	/* Handle QueryValue calls on HKEY_PERFORMANCE_DATA */
 	if(regkey->key->type == REG_KEY_HKPD)
@@ -287,6 +285,10 @@ WERROR _winreg_QueryValue(pipes_struct *p, struct winreg_QueryValue *r)
 		status = reg_queryvalue(p->mem_ctx, regkey, r->in.value_name->name,
 					&val);
 		if (!W_ERROR_IS_OK(status)) {
+
+			DEBUG(10,("_winreg_QueryValue: reg_queryvalue failed with: %s\n",
+				win_errstr(status)));
+
 			if (r->out.data_size) {
 				*r->out.data_size = 0;
 			}
@@ -306,19 +308,18 @@ WERROR _winreg_QueryValue(pipes_struct *p, struct winreg_QueryValue *r)
 		*r->out.type = val->type;
 	}
 
-	*r->out.data_length = outbuf_size;
+	status = WERR_BADFILE;
 
-	if ( *r->in.data_size == 0 || !r->out.data ) {
-		status = WERR_OK;
-	} else if ( *r->out.data_length > *r->in.data_size ) {
-		status = WERR_MORE_DATA;
+	if (*r->in.data_size < outbuf_size) {
+		*r->out.data_size = outbuf_size;
+		status = r->in.data ? WERR_MORE_DATA : WERR_OK;
 	} else {
-		memcpy( r->out.data, outbuf, *r->out.data_length );
+		*r->out.data_length = outbuf_size;
+		*r->out.data_size = outbuf_size;
+		memcpy(r->out.data, outbuf, outbuf_size);
 		status = WERR_OK;
 	}
 
-	*r->out.data_size = *r->out.data_length;
-
 	if (free_prs) prs_mem_free(&prs_hkpd);
 	if (free_buf) SAFE_FREE(outbuf);
 


-- 
Samba Shared Repository


More information about the samba-cvs mailing list