From 2ad2bdda89c07c0b8ce754c3b0cd4664eefc697d Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 26 May 2008 15:02:43 +1000 Subject: stricter checks for valid inputs in SMB2 open and lock (This used to be commit a7b5689a73adde59de28770aa3949660441291ea) --- source4/libcli/raw/interfaces.h | 1 + source4/libcli/raw/smb.h | 9 +++++++-- source4/ntvfs/ntvfs_generic.c | 18 ++++++++++++++++-- source4/ntvfs/posix/pvfs_open.c | 38 +++++++++++++++++++++++++++----------- source4/smb_server/smb/reply.c | 5 +++++ 5 files changed, 56 insertions(+), 15 deletions(-) diff --git a/source4/libcli/raw/interfaces.h b/source4/libcli/raw/interfaces.h index bae0e67b02..36d8c3abb0 100644 --- 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_ALL_MASK 0x00000017 uint32_t flags; uint32_t reserved; } *locks; diff --git a/source4/libcli/raw/smb.h b/source4/libcli/raw/smb.h index 74869e8a45..5a92b99757 100644 --- 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_MASK 7 /* 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_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 +#define NTCREATEX_OPTIONS_NOT_SUPPORTED_MASK 0x00DFA188 + + /* ntcreatex impersonation field */ #define NTCREATEX_IMPERSONATION_ANONYMOUS 0 diff --git a/source4/ntvfs/ntvfs_generic.c b/source4/ntvfs/ntvfs_generic.c index 62a1427405..9b4f235cde 100644 --- 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; + + /* 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; @@ -1031,6 +1037,9 @@ NTSTATUS ntvfs_map_lock(struct ntvfs_module_context *ntvfs, return NT_STATUS_NO_MEMORY; } for (i=0;ismb2.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++; @@ -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.data.length == 0 && - rd->smb2.in.length != 0) { + rd->smb2.out.data.length == 0) { 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; diff --git a/source4/ntvfs/posix/pvfs_open.c b/source4/ntvfs/posix/pvfs_open.c index 926c99d37e..59b42fe751 100644 --- 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; } + /* 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; @@ -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; } - + 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); } + 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); @@ -1152,16 +1178,6 @@ NTSTATUS pvfs_open(struct ntvfs_module_context *ntvfs, 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) { diff --git a/source4/smb_server/smb/reply.c b/source4/smb_server/smb/reply.c index 40cad91062..d28f4b6072 100644 --- 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; + /* 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)) { -- cgit