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
