stricter checks for valid inputs in SMB2 open and lock

(This used to be commit a7b5689a73adde59de28770aa3949660441291ea)

diff --git a/source4/libcli/raw/interfaces.h b/source4/libcli/raw/interfaces.h
index bae0e67b02e0b89ae359c9b3feae20c30c7fc6ad..36d8c3abb0a7c3237d3bacc4cf384027e636113a 100644 (file)
--- a/source4/libcli/raw/interfaces.h
+++ b/source4/libcli/raw/interfaces.h
@@ -1919,6 +1919,7 @@ union smb_lock {
 #define SMB2_LOCK_FLAG_EXCLUSIVE       0x00000002
 #define SMB2_LOCK_FLAG_UNLOCK          0x00000004
 #define SMB2_LOCK_FLAG_FAIL_IMMEDIATELY        0x00000010
 #define SMB2_LOCK_FLAG_EXCLUSIVE       0x00000002
 #define SMB2_LOCK_FLAG_UNLOCK          0x00000004
 #define SMB2_LOCK_FLAG_FAIL_IMMEDIATELY        0x00000010

+#define SMB2_LOCK_FLAG_ALL_MASK                0x00000017

                                uint32_t flags;
                                uint32_t reserved;
                        } *locks;
                                uint32_t flags;
                                uint32_t reserved;
                        } *locks;

diff --git a/source4/libcli/raw/smb.h b/source4/libcli/raw/smb.h
index 74869e8a45fed7aec00a53b9017edea81c90b741..5a92b99757043309112b976d0b730e42b53602cb 100644 (file)
--- a/source4/libcli/raw/smb.h
+++ b/source4/libcli/raw/smb.h
@@ -133,6 +133,7 @@
 #define NTCREATEX_SHARE_ACCESS_READ   1
 #define NTCREATEX_SHARE_ACCESS_WRITE  2
 #define NTCREATEX_SHARE_ACCESS_DELETE 4
 #define NTCREATEX_SHARE_ACCESS_READ   1
 #define NTCREATEX_SHARE_ACCESS_WRITE  2
 #define NTCREATEX_SHARE_ACCESS_DELETE 4

+#define NTCREATEX_SHARE_ACCESS_MASK   7

 
 /* ntcreatex open_disposition field */
 #define NTCREATEX_DISP_SUPERSEDE 0     /* supersede existing file (if it exists) */
 
 /* ntcreatex open_disposition field */
 #define NTCREATEX_DISP_SUPERSEDE 0     /* supersede existing file (if it exists) */


@@ -154,14 +155,18 @@
 #define NTCREATEX_OPTIONS_RANDOM_ACCESS            0x0800
 #define NTCREATEX_OPTIONS_DELETE_ON_CLOSE          0x1000
 #define NTCREATEX_OPTIONS_OPEN_BY_FILE_ID          0x2000
 #define NTCREATEX_OPTIONS_RANDOM_ACCESS            0x0800
 #define NTCREATEX_OPTIONS_DELETE_ON_CLOSE          0x1000
 #define NTCREATEX_OPTIONS_OPEN_BY_FILE_ID          0x2000

-#define NTCREATEX_OPTIONS_UNKNOWN_400000           0x400000
-
+#define NTCREATEX_OPTIONS_BACKUP_INTENT            0x4000
+#define NTCREATEX_OPTIONS_REPARSE_POINT          0x200000
+#define NTCREATEX_OPTIONS_UNKNOWN_400000         0x400000

 /* create options these bits are for private use by backends, they are
    not valid on the wire */
 #define NTCREATEX_OPTIONS_PRIVATE_MASK         0xFF000000
 #define NTCREATEX_OPTIONS_PRIVATE_DENY_DOS     0x01000000
 #define NTCREATEX_OPTIONS_PRIVATE_DENY_FCB     0x02000000
 
 /* create options these bits are for private use by backends, they are
    not valid on the wire */
 #define NTCREATEX_OPTIONS_PRIVATE_MASK         0xFF000000
 #define NTCREATEX_OPTIONS_PRIVATE_DENY_DOS     0x01000000
 #define NTCREATEX_OPTIONS_PRIVATE_DENY_FCB     0x02000000
 

+#define NTCREATEX_OPTIONS_NOT_SUPPORTED_MASK   0x00DFA188 
+
+

 
 /* ntcreatex impersonation field */
 #define NTCREATEX_IMPERSONATION_ANONYMOUS      0
 
 /* ntcreatex impersonation field */
 #define NTCREATEX_IMPERSONATION_ANONYMOUS      0

diff --git a/source4/ntvfs/ntvfs_generic.c b/source4/ntvfs/ntvfs_generic.c
index 62a142740536086206f475287d929270ba8bfcae..9b4f235cde151b5c53379cb3a5ccad524e76ddf9 100644 (file)
--- a/source4/ntvfs/ntvfs_generic.c
+++ b/source4/ntvfs/ntvfs_generic.c
@@ -522,6 +522,12 @@ NTSTATUS ntvfs_map_open(struct ntvfs_module_context *ntvfs,
                io2->generic.in.fname           = io->smb2.in.fname;
                io2->generic.in.sec_desc        = NULL;
                io2->generic.in.ea_list         = NULL;
                io2->generic.in.fname           = io->smb2.in.fname;
                io2->generic.in.sec_desc        = NULL;
                io2->generic.in.ea_list         = NULL;

+
+               /* we use a couple of bits of the create options internally */
+               if (io2->generic.in.create_options & NTCREATEX_OPTIONS_PRIVATE_MASK) {
+                       return NT_STATUS_INVALID_PARAMETER;
+               }
+

                status = ntvfs->ops->open(ntvfs, req, io2);             
                break;
 
                status = ntvfs->ops->open(ntvfs, req, io2);             
                break;
 


@@ -1031,6 +1037,9 @@ NTSTATUS ntvfs_map_lock(struct ntvfs_module_context *ntvfs,
                        return NT_STATUS_NO_MEMORY;
                }
                for (i=0;i<lck->smb2.in.lock_count;i++) {
                        return NT_STATUS_NO_MEMORY;
                }
                for (i=0;i<lck->smb2.in.lock_count;i++) {

+                       if (lck->smb2.in.locks[i].flags & ~SMB2_LOCK_FLAG_ALL_MASK) {
+                               return NT_STATUS_INVALID_PARAMETER;
+                       }

                        if (lck->smb2.in.locks[i].flags & SMB2_LOCK_FLAG_UNLOCK) {
                                int j = lck2->generic.in.ulock_cnt;
                                lck2->generic.in.ulock_cnt++;
                        if (lck->smb2.in.locks[i].flags & SMB2_LOCK_FLAG_UNLOCK) {
                                int j = lck2->generic.in.ulock_cnt;
                                lck2->generic.in.ulock_cnt++;


@@ -1277,10 +1286,15 @@ static NTSTATUS ntvfs_map_read_finish(struct ntvfs_module_context *ntvfs,
                rd->smb2.out.remaining  = 0;
                rd->smb2.out.reserved   = 0;
                if (NT_STATUS_IS_OK(status) &&
                rd->smb2.out.remaining  = 0;
                rd->smb2.out.reserved   = 0;
                if (NT_STATUS_IS_OK(status) &&

-                   rd->smb2.out.data.length == 0 &&
-                   rd->smb2.in.length != 0) {
+                   rd->smb2.out.data.length == 0) {

                        status = NT_STATUS_END_OF_FILE;
                }
                        status = NT_STATUS_END_OF_FILE;
                }

+               /* SMB2 does honor the min_count field, SMB does not */
+               if (NT_STATUS_IS_OK(status) && 
+                   rd->smb2.in.min_count > rd->smb2.out.data.length) {
+                       rd->smb2.out.data.length = 0;
+                       status = NT_STATUS_END_OF_FILE;                 
+               }

                break;
        default:
                return NT_STATUS_INVALID_LEVEL;
                break;
        default:
                return NT_STATUS_INVALID_LEVEL;

diff --git a/source4/ntvfs/posix/pvfs_open.c b/source4/ntvfs/posix/pvfs_open.c
index 926c99d37ed0c807fc20aa6eea5996e69d625a95..59b42fe751cf2ce6e628d295f9df829bbf23d3ee 100644 (file)
--- a/source4/ntvfs/posix/pvfs_open.c
+++ b/source4/ntvfs/posix/pvfs_open.c
@@ -203,6 +203,13 @@ static NTSTATUS pvfs_open_directory(struct pvfs_state *pvfs,
                return NT_STATUS_NOT_A_DIRECTORY;
        }
 
                return NT_STATUS_NOT_A_DIRECTORY;
        }
 

+       /* found with gentest */
+       if (io->ntcreatex.in.access_mask == SEC_FLAG_MAXIMUM_ALLOWED &&
+           (io->ntcreatex.in.create_options & NTCREATEX_OPTIONS_DIRECTORY) &&
+           (io->ntcreatex.in.create_options & NTCREATEX_OPTIONS_DELETE_ON_CLOSE)) {
+               return NT_STATUS_INVALID_PARAMETER;
+       }
+       

        switch (io->generic.in.open_disposition) {
        case NTCREATEX_DISP_OPEN_IF:
                break;
        switch (io->generic.in.open_disposition) {
        case NTCREATEX_DISP_OPEN_IF:
                break;


@@ -563,7 +570,7 @@ static NTSTATUS pvfs_create_file(struct pvfs_state *pvfs,
            (create_options & NTCREATEX_OPTIONS_DELETE_ON_CLOSE)) {
                return NT_STATUS_CANNOT_DELETE;
        }
            (create_options & NTCREATEX_OPTIONS_DELETE_ON_CLOSE)) {
                return NT_STATUS_CANNOT_DELETE;
        }

-       
+

        status = pvfs_access_check_create(pvfs, req, name, &access_mask);
        NT_STATUS_NOT_OK_RETURN(status);
 
        status = pvfs_access_check_create(pvfs, req, name, &access_mask);
        NT_STATUS_NOT_OK_RETURN(status);
 


@@ -1121,6 +1128,25 @@ NTSTATUS pvfs_open(struct ntvfs_module_context *ntvfs,
                return ntvfs_map_open(ntvfs, req, io);
        }
 
                return ntvfs_map_open(ntvfs, req, io);
        }
 

+       create_options = io->generic.in.create_options;
+       share_access   = io->generic.in.share_access;
+       access_mask    = io->generic.in.access_mask;
+
+       if (share_access & ~NTCREATEX_SHARE_ACCESS_MASK) {
+               return NT_STATUS_INVALID_PARAMETER;
+       }
+
+       /* some create options are not supported */
+       if (create_options & NTCREATEX_OPTIONS_NOT_SUPPORTED_MASK) {
+               return NT_STATUS_NOT_SUPPORTED;
+       }
+
+       /* other create options are not allowed */
+       if ((create_options & NTCREATEX_OPTIONS_DELETE_ON_CLOSE) &&
+           !(access_mask & SEC_STD_DELETE)) {
+               return NT_STATUS_INVALID_PARAMETER;
+       }
+

        /* resolve the cifs name to a posix name */
        status = pvfs_resolve_name(pvfs, req, io->ntcreatex.in.fname, 
                                   PVFS_RESOLVE_STREAMS, &name);
        /* resolve the cifs name to a posix name */
        status = pvfs_resolve_name(pvfs, req, io->ntcreatex.in.fname, 
                                   PVFS_RESOLVE_STREAMS, &name);


@@ -1152,16 +1178,6 @@ NTSTATUS pvfs_open(struct ntvfs_module_context *ntvfs,
           open doesn't match */
        io->generic.in.file_attr &= ~FILE_ATTRIBUTE_DIRECTORY;
 
           open doesn't match */
        io->generic.in.file_attr &= ~FILE_ATTRIBUTE_DIRECTORY;
 

-       create_options = io->generic.in.create_options;
-       share_access   = io->generic.in.share_access;
-       access_mask    = io->generic.in.access_mask;
-
-       /* certain create options are not allowed */
-       if ((create_options & NTCREATEX_OPTIONS_DELETE_ON_CLOSE) &&
-           !(access_mask & SEC_STD_DELETE)) {
-               return NT_STATUS_INVALID_PARAMETER;
-       }
-

        flags = 0;
 
        switch (io->generic.in.open_disposition) {
        flags = 0;
 
        switch (io->generic.in.open_disposition) {

diff --git a/source4/smb_server/smb/reply.c b/source4/smb_server/smb/reply.c
index 40cad91062b8b84681eec8519889a38dde049816..d28f4b6072e5abee0a37aa1b62a78a3195779df8 100644 (file)
--- a/source4/smb_server/smb/reply.c
+++ b/source4/smb_server/smb/reply.c
@@ -2193,6 +2193,11 @@ void smbsrv_reply_ntcreate_and_X(struct smbsrv_request *req)
        io->ntcreatex.in.ea_list          = NULL;
        io->ntcreatex.in.sec_desc         = NULL;
 
        io->ntcreatex.in.ea_list          = NULL;
        io->ntcreatex.in.sec_desc         = NULL;
 

+       /* we use a couple of bits of the create options internally */
+       if (io->ntcreatex.in.create_options & NTCREATEX_OPTIONS_PRIVATE_MASK) {
+               return NT_STATUS_INVALID_PARAMETER;
+       }
+

        /* we need a neater way to handle this alignment */
        if ((req->flags2 & FLAGS2_UNICODE_STRINGS) && 
            ucs2_align(req->in.buffer, req->in.data, STR_TERMINATE|STR_UNICODE)) {
        /* we need a neater way to handle this alignment */
        if ((req->flags2 & FLAGS2_UNICODE_STRINGS) && 
            ucs2_align(req->in.buffer, req->in.data, STR_TERMINATE|STR_UNICODE)) {