[PATCH 3/3] server: Check object's security when creating handles.

[Robert Shearman]

Vitaliy Margolen wrote:
> After
> checking object's SD against token we fail some tests.
> What it seems to me is that some one tried to "optimize" this part in
> windows and instead created a security problem.

If you already have a valid handle to the object, then it isn't really a 
security problem to have another one with the same access rights.

> @@ -450,25 +453,34 @@ obj_handle_t duplicate_handle( struct pr
>                                 unsigned int access, unsigned int attr, unsigned int options )
>  {
>      obj_handle_t res;
> +    struct handle_entry *entry;
> +    unsigned int src_access, dst_access;
>      struct object *obj = get_handle_obj( src, src_handle, 0, NULL );
>  
>      if (!obj) return 0;
> -    if (options & DUP_HANDLE_SAME_ACCESS)
> +    if ((entry = get_handle( src, src_handle )))
> +        src_access = entry->access;
> +    else  /* pseudo-handle, give it full access */
>      {
> -        struct handle_entry *entry = get_handle( src, src_handle );
> -        if (entry)
> -            access = entry->access;
> -        else  /* pseudo-handle, give it full access */
> -        {
> -            access = obj->ops->map_access( obj, GENERIC_ALL );
> -            clear_error();
> -        }
> +        src_access = obj->ops->map_access( obj, GENERIC_ALL );
> +        clear_error();
>      }
> -    access &= ~RESERVED_ALL;
> +    src_access &= ~RESERVED_ALL;
> +
> +    if (options & DUP_HANDLE_SAME_ACCESS)
> +        dst_access = src_access;
> +    else
> +        dst_access = obj->ops->map_access( obj, access ) & ~RESERVED_ALL;
> +
> +    /* asking for more or the same/less access rights */
> +    access = ~src_access & dst_access ? dst_access : 0;
> +
>      if (options & DUP_HANDLE_MAKE_GLOBAL)
>          res = alloc_global_handle( obj, access );
>      else
>          res = alloc_handle( dst, obj, access, attr );
> +    if (res) get_handle( dst, res )->access = dst_access;
> +
>   

It would seem better to add an extra parameter to alloc_handle and 
alloc_global_handle to tell them to not check access rather than adding 
this hack. It should use less processor time and it should make patch 2 
not necessary.


-- 
Rob Shearman