No access check verification on stream files.
Jeremy Allison
jra at samba.org
Mon Oct 28 18:02:43 MDT 2013
On Fri, Oct 25, 2013 at 04:48:54PM +0530, Hemanth Thummala wrote:
> Hi All,
>
> Looks like there are no ACL checks against stream files while performing
> generic file operations like read and write.
>
> For example:
> we have created a stream file on remote CIFS share using domain
> administrator as follows.
>
> C:\>echo "NON-Default stream file" > z:\non-def-stream
> .txt:stream1
>
> C:\>more < z:\non-def-stream.txt:stream1
> "NON-Default stream file"
>
> After that we have added explicit deny write ACE for a specific domain
> user.
>
> But that user was still be able to access and write the data to this stream
> file thought he has an explicit deny permission.
>
> Whereas this issue is not seen with $DATA streams.
>
> >From the code, I could see whenever create request comes for stream file,
> we trying to open the base file with access mask as zero..
>
> /* Open the base file. */
> status = create_file_unixpath(conn, NULL,
> smb_fname_base, 0, ===> Access mask is zero.
> FILE_SHARE_READ
> | FILE_SHARE_WRITE
> | FILE_SHARE_DELETE,
> base_create_disposition,
> 0, 0, 0, 0, 0, NULL, NULL,
> &base_fsp, NULL);
>
>
> And of course lot other parameters were sent as zero. FSP coming out
> of this getting assigned to fsp->base_fsp.
>
> In open_acl_common(), if fsp->base_fsp is non null then we are
> skipping the ACL check claiming we already did on base file. But while
> doing on base file, we used access mask as zero and it is not
> evaluated for original access request from client. Here is the code
> snippet in open_acl_common() which does this..
>
> if (fsp->base_fsp) {
> /* Stream open. Base filename open already did the ACL check. */
> DEBUG(10,("open_acl_common: stream open on %s\n",
> fsp_str_dbg(fsp) ));
> return SMB_VFS_NEXT_OPEN(handle, smb_fname, fsp, flags, mode);
> }
>
>
> In my case, problem is not see after using the original access mask in
> create_file_unixpath() for base file.
>
> Here is the simple change that worked for me.
>
> bash-4.0$ diff -up samba-3.6.12/source3/smbd/open.c.orig
> samba-3.6.12/source3/smbd/open.c
> --- samba-3.6.12/source3/smbd/open.c.orig 2013-10-25
> 03:49:03.651630453 -0700
> +++ samba-3.6.12/source3/smbd/open.c 2013-10-25 03:48:46.243898186 -0700
> @@ -3262,7 +3262,7 @@ static NTSTATUS create_file_unixpath(con
> }
>
> /* Open the base file. */
> - status = create_file_unixpath(conn, NULL, smb_fname_base, 0,
> + status = create_file_unixpath(conn, NULL,
> smb_fname_base, access_mask,
> FILE_SHARE_READ
> | FILE_SHARE_WRITE
> | FILE_SHARE_DELETE,
>
>
> Please let me know if this is the correct thing to do.
I think the following patch is the correct one, as it
passes our regression test suite (whereas yours unfortunately
does not).
I'm going to add a regression test for this and get it pushed
into master, and then create back-ports for 3.6.x and above.
Thanks !
Jeremy.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-bug-10229-No-access-check-verification-on-stream.patch
Type: text/x-diff
Size: 3008 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20131028/3f1fc04c/attachment.patch>
More information about the samba-technical
mailing list