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 <g...@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 <g...@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 <g...@samba.org>
Date: Wed Mar 17 12:06:39 2010 +0100
s3: re-run make samba3-idl.
Guenther
commit f9a52643a5665efd5b11733179e574e3d0282e8b
Author: Günther Deschner <g...@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
