v3: Check the length and reject invalid root name.
Signed-off-by: Dmitry Timoshkov dmitry@baikal.ru --- dlls/ntdll/tests/reg.c | 2 -- server/registry.c | 62 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 56 insertions(+), 8 deletions(-)
diff --git a/dlls/ntdll/tests/reg.c b/dlls/ntdll/tests/reg.c index 91b752ac069..74d82b85be4 100644 --- a/dlls/ntdll/tests/reg.c +++ b/dlls/ntdll/tests/reg.c @@ -417,7 +417,6 @@ todo_wine
pRtlCreateUnicodeStringFromAsciiz( &str, "\Registry" ); status = pNtOpenKey(&key, KEY_READ, &attr); - todo_wine ok( status == STATUS_SUCCESS, "NtOpenKey failed: 0x%08x\n", status ); pNtClose( key ); pRtlFreeUnicodeString( &str ); @@ -569,7 +568,6 @@ static void test_NtCreateKey(void)
pRtlCreateUnicodeStringFromAsciiz( &str, "\Registry" ); status = pNtCreateKey( &subkey, am, &attr, 0, 0, 0, 0 ); - todo_wine ok( status == STATUS_SUCCESS || status == STATUS_ACCESS_DENIED, "NtCreateKey failed: 0x%08x\n", status ); if (!status) pNtClose( subkey ); diff --git a/server/registry.c b/server/registry.c index b00abdbc004..4ed3ee9896e 100644 --- a/server/registry.c +++ b/server/registry.c @@ -127,7 +127,7 @@ static const timeout_t save_period = 30 * -TICKS_PER_SEC; /* delay between peri static struct timeout_user *save_timeout_user; /* saving timer */ static enum prefix_type { PREFIX_UNKNOWN, PREFIX_32BIT, PREFIX_64BIT } prefix_type;
-static const WCHAR root_name[] = { '\','R','e','g','i','s','t','r','y','\' }; +static const WCHAR root_name[] = { '\','R','e','g','i','s','t','r','y' }; static const WCHAR wow6432node[] = {'W','o','w','6','4','3','2','N','o','d','e'}; static const WCHAR symlink_value[] = {'S','y','m','b','o','l','i','c','L','i','n','k','V','a','l','u','e'}; static const struct unicode_str symlink_str = { symlink_value, sizeof(symlink_value) }; @@ -411,7 +411,7 @@ static WCHAR *key_get_full_name( struct object *obj, data_size_t *ret_len ) { static const WCHAR backslash = '\'; struct key *key = (struct key *) obj; - data_size_t len = sizeof(root_name) - sizeof(WCHAR); + data_size_t len = sizeof(root_name); char *ret;
if (key->flags & KEY_DELETED) @@ -431,7 +431,7 @@ static WCHAR *key_get_full_name( struct object *obj, data_size_t *ret_len ) len -= key->namelen + sizeof(WCHAR); memcpy( ret + len, &backslash, sizeof(WCHAR) ); } - memcpy( ret, root_name, sizeof(root_name) - sizeof(WCHAR) ); + memcpy( ret, root_name, sizeof(root_name) ); return (WCHAR *)ret; }
@@ -474,7 +474,7 @@ static void key_destroy( struct object *obj ) }
/* get the request vararg as registry path */ -static inline void get_req_path( struct unicode_str *str, int skip_root ) +static inline struct unicode_str *get_req_path( struct unicode_str *str, int skip_root ) { str->str = get_req_data(); str->len = (get_req_data_size() / sizeof(WCHAR)) * sizeof(WCHAR); @@ -483,7 +483,19 @@ static inline void get_req_path( struct unicode_str *str, int skip_root ) { str->str += ARRAY_SIZE( root_name ); str->len -= sizeof(root_name); + if (str->len) + { + if (str->str[0] != '\') + { + set_error( STATUS_OBJECT_PATH_INVALID ); + return NULL; + } + + str->str++; + str->len -= sizeof(WCHAR); + } } + return str; }
/* return the next token in a given path */ @@ -734,6 +746,12 @@ static struct key *follow_symlink( struct key *key, int iteration ) if (memicmp_strW( path.str, root_name, sizeof(root_name) )) return NULL; path.str += ARRAY_SIZE( root_name ); path.len -= sizeof(root_name); + if (path.len) + { + if (path.str[0] != '\') return NULL; + path.str++; + path.len -= sizeof(WCHAR); + }
key = root_key; token.str = NULL; @@ -2156,6 +2174,17 @@ DECL_HANDLER(create_key) { name.str += ARRAY_SIZE( root_name ); name.len -= sizeof(root_name); + if (name.len) + { + if (name.str[0] != '\') + { + set_error( STATUS_OBJECT_PATH_INVALID ); + return; + } + + name.str++; + name.len -= sizeof(WCHAR); + } }
/* NOTE: no access rights are required from the parent handle to create a key */ @@ -2184,7 +2213,12 @@ DECL_HANDLER(open_key) /* NOTE: no access rights are required to open the parent key, only the child key */ if ((parent = get_parent_hkey_obj( req->parent ))) { - get_req_path( &name, !req->parent ); + if (!get_req_path( &name, !req->parent )) + { + release_object( parent ); + return; + } + if ((key = open_key( parent, &name, access, req->attributes ))) { reply->hkey = alloc_handle( current->process, key, access, req->attributes ); @@ -2314,6 +2348,17 @@ DECL_HANDLER(load_registry) { name.str += ARRAY_SIZE( root_name ); name.len -= sizeof(root_name); + if (name.len) + { + if (name.str[0] != '\') + { + set_error( STATUS_OBJECT_PATH_INVALID ); + return; + } + + name.str++; + name.len -= sizeof(WCHAR); + } }
if ((parent = get_parent_hkey_obj( objattr->rootdir ))) @@ -2344,7 +2389,12 @@ DECL_HANDLER(unload_registry)
if ((parent = get_parent_hkey_obj( req->parent ))) { - get_req_path( &name, !req->parent ); + if (!get_req_path( &name, !req->parent )) + { + release_object( parent ); + return; + } + if ((key = open_key( parent, &name, access, req->attributes ))) { if (key->obj.handle_count)
Hello,
can I get a review of these patches please?
Dmitry Timoshkov dmitry@baikal.ru writes:
Hello,
can I get a review of these patches please?
As already mentioned, please try to make this part of the normal path handling.
-
Alexandre Julliard -
Dmitry Timoshkov