smb2_file_rename_information must check that the file has been opened with DELETE Access ...

[Richard Sharpe]

On Wed, Nov 14, 2012 at 4:44 PM, Jeremy Allison <jra at samba.org> wrote:
> On Tue, Nov 13, 2012 at 08:38:03PM -0800, Richard Sharpe wrote:
>> Hi folks,
>>
>> One of the smb2 tests shows that you can only do a SET_FILE_INFO
>> Rename Info if you have the source file open for delete.
>>
>> This is confirmed by MS-FSCC section x.y.z. I will confirm the
>> specific section tomorrow.
>>
>> The following is a potential fix.
>>
>> diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
>> index 61d755c..d919ad4 100644
>> --- a/source3/smbd/trans2.c
>> +++ b/source3/smbd/trans2.c
>> @@ -6100,6 +6100,9 @@ static NTSTATUS smb2_file_rename_information(connection_st
>>                 return NT_STATUS_INVALID_PARAMETER;
>>         }
>>
>> +       if (!(fsp->access_mask & DELETE_ACCESS)) {
>> +               return NT_STATUS_ACCESS_DENIED;
>> +       }
>>         srvstr_get_path(ctx, pdata, req->flags2, &newname,
>>                                 &pdata[20], len, STR_TERMINATE,
>>                                 &status);
>
> Hmmm. Are you sure we fail this already ?
>
> Check out the function can_rename() in source3/smbd/reply.c.
>
> It has:
>
>         if (fsp->access_mask & (DELETE_ACCESS|FILE_WRITE_ATTRIBUTES)) {
>                 return NT_STATUS_OK;
>         }
>
>         return NT_STATUS_ACCESS_DENIED;
>
> Aha! That probably should be:
>
>         if ((fsp->access_mask & (DELETE_ACCESS|FILE_WRITE_ATTRIBUTES)) ==
>                         (DELETE_ACCESS|FILE_WRITE_ATTRIBUTES)) {
>                 return NT_STATUS_OK;
>         }
>
>         return NT_STATUS_ACCESS_DENIED;
>
> I think that's the correct fix for smb1 and smb2.

Ahhh, OK. that looks better. I was testing against 3.6.6 and did not
look far enough for the fix.

I will try this out soon as my build completes ...

-- 
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)