From 2e4e06c6e69d881b0fc3c53df50db607fcce4a91 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Sat, 16 Feb 2008 07:25:38 +1100 Subject: fixed handling of zero sized buffers versus NULL buffers in SMB2. Thanks to Metze for spotting this. (This used to be commit fbcf3e65b9284e5d1862c98706d7f148a36afe47) --- source4/libcli/smb2/request.c | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/source4/libcli/smb2/request.c b/source4/libcli/smb2/request.c index 7a0311f886..1de0531d9a 100644 --- a/source4/libcli/smb2/request.c +++ b/source4/libcli/smb2/request.c @@ -206,6 +206,10 @@ bool smb2_request_is_ok(struct smb2_request *req) */ bool smb2_oob(struct smb2_request_buffer *buf, const uint8_t *ptr, size_t size) { + if (size == 0) { + /* zero bytes is never out of range */ + return false; + } /* be careful with wraparound! */ if (ptr < buf->body || ptr >= buf->body + buf->body_size || @@ -270,7 +274,7 @@ NTSTATUS smb2_pull_o16s16_blob(struct smb2_request_buffer *buf, TALLOC_CTX *mem_ } ofs = SVAL(ptr, 0); size = SVAL(ptr, 2); - if (ofs == 0 || size == 0) { + if (ofs == 0) { *blob = data_blob(NULL, 0); return NT_STATUS_OK; } @@ -310,7 +314,10 @@ NTSTATUS smb2_push_o16s16_blob(struct smb2_request_buffer *buf, return NT_STATUS_BUFFER_TOO_SMALL; } - if (blob.length == 0) { + if (blob.data == NULL) { + if (blob.length != 0) { + return NT_STATUS_INTERNAL_ERROR; + } SSVAL(ptr, 0, 0); SSVAL(ptr, 2, 0); return NT_STATUS_OK; @@ -363,7 +370,10 @@ NTSTATUS smb2_push_o16s32_blob(struct smb2_request_buffer *buf, return NT_STATUS_BUFFER_TOO_SMALL; } - if (blob.length == 0) { + if (blob.data == NULL) { + if (blob.length != 0) { + return NT_STATUS_INTERNAL_ERROR; + } SSVAL(ptr, 0, 0); SIVAL(ptr, 2, 0); return NT_STATUS_OK; @@ -416,7 +426,10 @@ NTSTATUS smb2_push_o32s32_blob(struct smb2_request_buffer *buf, return NT_STATUS_BUFFER_TOO_SMALL; } - if (blob.length == 0) { + if (blob.data == NULL) { + if (blob.length != 0) { + return NT_STATUS_INTERNAL_ERROR; + } SIVAL(ptr, 0, 0); SIVAL(ptr, 4, 0); return NT_STATUS_OK; @@ -469,7 +482,10 @@ NTSTATUS smb2_push_s32o32_blob(struct smb2_request_buffer *buf, return NT_STATUS_BUFFER_TOO_SMALL; } - if (blob.length == 0) { + if (blob.data == NULL) { + if (blob.length != 0) { + return NT_STATUS_INTERNAL_ERROR; + } SIVAL(ptr, 0, 0); SIVAL(ptr, 4, 0); return NT_STATUS_OK; @@ -512,7 +528,7 @@ NTSTATUS smb2_pull_o16s32_blob(struct smb2_request_buffer *buf, TALLOC_CTX *mem_ } ofs = SVAL(ptr, 0); size = IVAL(ptr, 2); - if (ofs == 0 || size == 0) { + if (ofs == 0) { *blob = data_blob(NULL, 0); return NT_STATUS_OK; } @@ -536,7 +552,7 @@ NTSTATUS smb2_pull_o32s32_blob(struct smb2_request_buffer *buf, TALLOC_CTX *mem_ } ofs = IVAL(ptr, 0); size = IVAL(ptr, 4); - if (ofs == 0 || size == 0) { + if (ofs == 0) { *blob = data_blob(NULL, 0); return NT_STATUS_OK; } @@ -563,7 +579,7 @@ NTSTATUS smb2_pull_o16As32_blob(struct smb2_request_buffer *buf, TALLOC_CTX *mem } ofs = SVAL(ptr, 0); size = IVAL(ptr, 4); - if (ofs == 0 || size == 0) { + if (ofs == 0) { *blob = data_blob(NULL, 0); return NT_STATUS_OK; } @@ -587,7 +603,7 @@ NTSTATUS smb2_pull_s32o32_blob(struct smb2_request_buffer *buf, TALLOC_CTX *mem_ } size = IVAL(ptr, 0); ofs = IVAL(ptr, 4); - if (ofs == 0 || size == 0) { + if (ofs == 0) { *blob = data_blob(NULL, 0); return NT_STATUS_OK; } @@ -614,6 +630,11 @@ NTSTATUS smb2_pull_o16s16_string(struct smb2_request_buffer *buf, TALLOC_CTX *me status = smb2_pull_o16s16_blob(buf, mem_ctx, ptr, &blob); NT_STATUS_NOT_OK_RETURN(status); + if (blob.data == NULL) { + *str = NULL; + return NT_STATUS_OK; + } + if (blob.length == 0) { char *s; s = talloc_strdup(mem_ctx, ""); @@ -643,7 +664,7 @@ NTSTATUS smb2_push_o16s16_string(struct smb2_request_buffer *buf, NTSTATUS status; ssize_t size; - if (strcmp("", str) == 0) { + if (str == NULL) { return smb2_push_o16s16_blob(buf, ofs, data_blob(NULL, 0)); } -- cgit From afe8e5551e41550f40271f49ab4ded5e5f0def9c Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Sat, 16 Feb 2008 13:28:37 +1100 Subject: fixed RAW-READ after the bufinfo changes. Thanks to Metze for spotting this. (This used to be commit 3c9973b695a0b5c30d3a5bfabecf62dd1a25ebc1) --- source4/libcli/raw/rawreadwrite.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source4/libcli/raw/rawreadwrite.c b/source4/libcli/raw/rawreadwrite.c index 2005e36e04..9e4edaf99c 100644 --- a/source4/libcli/raw/rawreadwrite.c +++ b/source4/libcli/raw/rawreadwrite.c @@ -171,6 +171,9 @@ NTSTATUS smb_raw_read_recv(struct smbcli_request *req, union smb_read *parms) parms->readx.out.nread <= req->in.buffer + req->in.size) { req->in.data_size += (SVAL(req->in.vwv, VWV(7)) << 16); + + /* update the bufinfo with the new size */ + smb_setup_bufinfo(req); } } -- cgit From 26b8701321909e10aac124324695f9f4891b1f8d Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 18 Feb 2008 14:53:48 +1100 Subject: handle pushing of zero length smb2 strings (This used to be commit 66d0502228b31533b5d93731128a681992c22eda) --- source4/libcli/smb2/request.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/source4/libcli/smb2/request.c b/source4/libcli/smb2/request.c index 1de0531d9a..2471fcaa4d 100644 --- a/source4/libcli/smb2/request.c +++ b/source4/libcli/smb2/request.c @@ -668,6 +668,12 @@ NTSTATUS smb2_push_o16s16_string(struct smb2_request_buffer *buf, return smb2_push_o16s16_blob(buf, ofs, data_blob(NULL, 0)); } + if (*str == 0) { + blob.data = str; + blob.length = 0; + return smb2_push_o16s16_blob(buf, ofs, blob); + } + size = convert_string_talloc(buf->buffer, lp_iconv_convenience(global_loadparm), CH_UNIX, CH_UTF16, str, strlen(str), (void **)&blob.data); if (size == -1) { -- cgit From 5fb241e5d33024dd7af1e7ec5cbdefbf144be6a1 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 18 Feb 2008 14:54:14 +1100 Subject: 3 places where the VFS backend doesn't handle NULL strings. (This used to be commit cf109460aff5a8437ab7eba05e4d7316a131080e) --- source4/smb_server/smb2/fileio.c | 5 +++++ source4/smb_server/smb2/find.c | 5 +++++ source4/smb_server/smb2/tcon.c | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/source4/smb_server/smb2/fileio.c b/source4/smb_server/smb2/fileio.c index 567243ba94..0e3df56b42 100644 --- a/source4/smb_server/smb2/fileio.c +++ b/source4/smb_server/smb2/fileio.c @@ -80,6 +80,11 @@ void smb2srv_create_recv(struct smb2srv_request *req) /* TODO: parse the blob */ ZERO_STRUCT(io->smb2.in.eas); + /* the VFS backend does not yet handle NULL filenames */ + if (io->smb2.in.fname == NULL) { + io->smb2.in.fname = ""; + } + SMB2SRV_CALL_NTVFS_BACKEND(ntvfs_open(req->ntvfs, io)); } diff --git a/source4/smb_server/smb2/find.c b/source4/smb_server/smb2/find.c index c594adf7a0..6018f1958f 100644 --- a/source4/smb_server/smb2/find.c +++ b/source4/smb_server/smb2/find.c @@ -161,6 +161,11 @@ void smb2srv_find_recv(struct smb2srv_request *req) SMB2SRV_CHECK(smb2_pull_o16s16_string(&req->in, info, req->in.body+0x18, &info->in.pattern)); info->in.max_response_size = IVAL(req->in.body, 0x1C); + /* the VFS backend does not yet handle NULL patterns */ + if (info->in.pattern == NULL) { + info->in.pattern = ""; + } + SMB2SRV_CHECK_FILE_HANDLE(info->in.file.ntvfs); SMB2SRV_CALL_NTVFS_BACKEND(smb2srv_find_backend(state)); } diff --git a/source4/smb_server/smb2/tcon.c b/source4/smb_server/smb2/tcon.c index 50094b806d..7f7d558b16 100644 --- a/source4/smb_server/smb2/tcon.c +++ b/source4/smb_server/smb2/tcon.c @@ -394,6 +394,11 @@ void smb2srv_tcon_recv(struct smb2srv_request *req) io->smb2.in.reserved = SVAL(req->in.body, 0x02); SMB2SRV_CHECK(smb2_pull_o16s16_string(&req->in, io, req->in.body+0x04, &io->smb2.in.path)); + /* the VFS backend does not yet handle NULL paths */ + if (io->smb2.in.path == NULL) { + io->smb2.in.path = ""; + } + req->status = smb2srv_tcon_backend(req, io); if (req->control_flags & SMB2SRV_REQ_CTRL_FLAG_NOT_REPLY) { -- cgit From 10a374421bac7df15ab398bc46580be0d06d9d7d Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 18 Feb 2008 14:54:59 +1100 Subject: open a root handle in SMB2 should use a NULL filename, not a zero length filename (This used to be commit a29dd708bf26440552ffa9d83332329b4c108857) --- source4/torture/smb2/util.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source4/torture/smb2/util.c b/source4/torture/smb2/util.c index 219c2140d3..f85b1c42ff 100644 --- a/source4/torture/smb2/util.c +++ b/source4/torture/smb2/util.c @@ -123,6 +123,7 @@ static NTSTATUS smb2_create_complex(struct smb2_tree *tree, const char *fname, io.in.create_disposition = NTCREATEX_DISP_CREATE; } + /* it seems vista is now fussier about alignment? */ if (strchr(fname, ':') == NULL) { /* setup some EAs */ io.in.eas.num_eas = 2; @@ -428,7 +429,7 @@ NTSTATUS smb2_util_roothandle(struct smb2_tree *tree, struct smb2_handle *handle io.in.create_disposition = NTCREATEX_DISP_OPEN; io.in.share_access = NTCREATEX_SHARE_ACCESS_READ|NTCREATEX_SHARE_ACCESS_DELETE; io.in.create_options = NTCREATEX_OPTIONS_ASYNC_ALERT; - io.in.fname = ""; + io.in.fname = NULL; status = smb2_create(tree, tree, &io); NT_STATUS_NOT_OK_RETURN(status); -- cgit From 0e03c3aa892439bb5567b0129a40b267ecb6cee5 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 18 Feb 2008 14:55:30 +1100 Subject: disable the EAS level in SMB2-GETINFO test until we get some feedback on how the alignment requirements have changed (This used to be commit 2a474568c2f85603657a97ad658089122a1f4f19) --- source4/torture/smb2/getinfo.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/source4/torture/smb2/getinfo.c b/source4/torture/smb2/getinfo.c index f561b62d47..c47a26277c 100644 --- a/source4/torture/smb2/getinfo.c +++ b/source4/torture/smb2/getinfo.c @@ -51,7 +51,9 @@ static struct { { LEVEL(RAW_FILEINFO_COMPRESSION_INFORMATION) }, { LEVEL(RAW_FILEINFO_NETWORK_OPEN_INFORMATION) }, { LEVEL(RAW_FILEINFO_ATTRIBUTE_TAG_INFORMATION) }, - { LEVEL(RAW_FILEINFO_SMB2_ALL_EAS) }, +/* +disabled until we know how the alignment now works +{ LEVEL(RAW_FILEINFO_SMB2_ALL_EAS) }, */ { LEVEL(RAW_FILEINFO_SMB2_ALL_INFORMATION) }, { LEVEL(RAW_FILEINFO_SEC_DESC) } }; @@ -85,13 +87,13 @@ static bool torture_smb2_fileinfo(struct torture_context *tctx, struct smb2_tree status = torture_smb2_testfile(tree, FNAME, &hfile); if (!NT_STATUS_IS_OK(status)) { - printf("Unable to create test file '%s' - %s\n", FNAME, nt_errstr(status)); + printf(__location__ " Unable to create test file '%s' - %s\n", FNAME, nt_errstr(status)); goto failed; } status = torture_smb2_testdir(tree, DNAME, &hdir); if (!NT_STATUS_IS_OK(status)) { - printf("Unable to create test directory '%s' - %s\n", DNAME, nt_errstr(status)); + printf(__location__ " Unable to create test directory '%s' - %s\n", DNAME, nt_errstr(status)); goto failed; } @@ -150,7 +152,7 @@ static bool torture_smb2_fsinfo(struct smb2_tree *tree) printf("Testing fsinfo levels\n"); status = smb2_util_roothandle(tree, &handle); if (!NT_STATUS_IS_OK(status)) { - printf("Unable to create test directory '%s' - %s\n", DNAME, nt_errstr(status)); + printf(__location__ " Unable to create root handle - %s\n", nt_errstr(status)); return false; } -- cgit