[PATCH v3 1/2] server: Terminating '\' in the registry root name is optional.

27 Dec 2021

v3: Check the length and reject invalid root name.

Signed-off-by: Dmitry Timoshkov <dmitry(a)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)
-- 
2.34.1

    

Dmitry Timoshkov

Dmitry Timoshkov

Alexandre Julliard

tags

participants (2)