From 39eb7e2bcc0c41dfefc8a4ee01b22ec09591afc4 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 25 Jun 2015 15:42:04 +0200 Subject: [PATCH 1/2] vfs_fruit: check offset and length for AFP_AfpInfo read requests fruit_pread doesn't check the offset and length parameters and instead always writes 60 bytes, the size of the AFP_AfpInfo blob, to the the passed buffer. If the passed in buffer is smaller, we overwrite something somewhere. Bug: https://bugzilla.samba.org/show_bug.cgi?id=11363 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index a4272f5..74cbc71 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -2621,6 +2621,21 @@ static ssize_t fruit_pread(vfs_handle_struct *handle, } if (ad->ad_type == ADOUBLE_META) { + char afpinfo_buf[AFP_INFO_SIZE]; + size_t to_return; + + if (offset > AFP_INFO_SIZE) { + len = 0; + rc = 0; + goto exit; + } + + if (offset + n > AFP_INFO_SIZE) { + to_return = AFP_INFO_SIZE - offset; + } else { + to_return = n; + } + ai = afpinfo_new(talloc_tos()); if (ai == NULL) { rc = -1; @@ -2636,11 +2651,14 @@ static ssize_t fruit_pread(vfs_handle_struct *handle, memcpy(&ai->afpi_FinderInfo[0], ad_entry(ad, ADEID_FINDERI), ADEDLEN_FINDERI); - len = afpinfo_pack(ai, data); + len = afpinfo_pack(ai, afpinfo_buf); if (len != AFP_INFO_SIZE) { rc = -1; goto exit; } + + memcpy(data, afpinfo_buf + offset, to_return); + len = to_return; } else { len = SMB_VFS_NEXT_PREAD( handle, fsp, data, n, -- 2.1.0 From 595e8c8a468c9e60d8e098247857c5ed57d61eef Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 25 Jun 2015 16:25:05 +0200 Subject: [PATCH 2/2] s4:torture:vfs_fruit: check offset and length when reading AFP_AfpInfo stream Bug: https://bugzilla.samba.org/show_bug.cgi?id=11363 Signed-off-by: Ralph Boehme --- source4/torture/vfs/fruit.c | 88 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index c65ba74..9af7fc8 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -49,6 +49,15 @@ goto done; \ }} while (0) +#define CHECK_VALUE(v, correct) do { \ + if ((v) != (correct)) { \ + torture_result(tctx, TORTURE_FAIL, \ + "(%s) Incorrect value %s=%u - should be %u\n", \ + __location__, #v, (unsigned)v, (unsigned)correct); \ + ret = false; \ + goto done; \ + }} while (0) + /* * REVIEW: * This is hokey, but what else can we do? @@ -932,6 +941,67 @@ static bool check_stream(struct smb2_tree *tree, * Read 'count' bytes at 'offset' from stream 'fname:sname' and * compare against buffer 'value' **/ +static ssize_t read_stream(struct smb2_tree *tree, + const char *location, + struct torture_context *tctx, + TALLOC_CTX *mem_ctx, + const char *fname, + const char *sname, + off_t read_offset, + size_t read_count) +{ + struct smb2_handle handle; + struct smb2_create create; + struct smb2_read r; + NTSTATUS status; + const char *full_name; + bool ret = true; + + full_name = talloc_asprintf(mem_ctx, "%s%s", fname, sname); + if (full_name == NULL) { + torture_comment(tctx, "talloc_asprintf error\n"); + return -1; + } + ZERO_STRUCT(create); + create.in.desired_access = SEC_FILE_READ_DATA; + create.in.file_attributes = FILE_ATTRIBUTE_NORMAL; + create.in.create_disposition = NTCREATEX_DISP_OPEN; + create.in.fname = full_name; + + torture_comment(tctx, "Open stream %s\n", full_name); + + status = smb2_create(tree, mem_ctx, &create); + if (!NT_STATUS_IS_OK(status)) { + torture_comment(tctx, "Unable to open stream %s\n", + full_name); + return -1; + } + + handle = create.out.file.handle; + + ZERO_STRUCT(r); + r.in.file.handle = handle; + r.in.length = read_count; + r.in.offset = read_offset; + + status = smb2_read(tree, tree, &r); + if (!NT_STATUS_IS_OK(status)) { + CHECK_STATUS(status, NT_STATUS_END_OF_FILE); + } + + smb2_util_close(tree, handle); + +done: + if (ret == false) { + return -1; + } + return r.out.data.length; +} + +/** + * Read 'count' bytes at 'offset' from stream 'fname:sname' and + * compare against buffer 'value' + **/ static bool write_stream(struct smb2_tree *tree, const char *location, struct torture_context *tctx, @@ -1108,6 +1178,7 @@ static bool test_read_atalk_metadata(struct torture_context *tctx, NTSTATUS status; struct smb2_handle testdirh; bool ret = true; + ssize_t len; torture_comment(tctx, "Checking metadata access\n"); @@ -1135,6 +1206,23 @@ static bool test_read_atalk_metadata(struct torture_context *tctx, ret &= check_stream(tree1, __location__, tctx, mem_ctx, fname, AFPINFO_STREAM, 0, 60, 16, 8, "BARRFOOO"); + ret &= check_stream(tree1, __location__, tctx, mem_ctx, fname, AFPINFO_STREAM, + 16, 8, 0, 8, "BARRFOOO"); + + /* Check reading offset and read size > sizeof(AFPINFO_STREAM) */ + + len = read_stream(tree1, __location__, tctx, mem_ctx, fname, + AFPINFO_STREAM, 0, 61); + CHECK_VALUE(len, 60); + + len = read_stream(tree1, __location__, tctx, mem_ctx, fname, + AFPINFO_STREAM, 59, 2); + CHECK_VALUE(len, 1); + + len = read_stream(tree1, __location__, tctx, mem_ctx, fname, + AFPINFO_STREAM, 61, 1); + CHECK_VALUE(len, 0); + done: smb2_deltree(tree1, BASEDIR); talloc_free(mem_ctx); -- 2.1.0