Attachment 11224 Details for Bug 11363 – Patch for master

[patch] Patch for master

fruit.patch (text/plain), 5.44 KB, created by Ralph Böhme on 2015-07-02 10:16:19 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2015-07-02 10:16:19 UTC

Size: 5.44 KB

patch

obsolete

>From a73019fefcbc3541dfd2348682222d7ccbd0ab1f Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>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 <slow@samba.org>
>---
> source3/modules/vfs_fruit.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
>diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
>index a4272f5..d05d786 100644
>--- a/source3/modules/vfs_fruit.c
>+++ b/source3/modules/vfs_fruit.c
>@@ -2621,6 +2621,17 @@ 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 < 0) || (offset > AFP_INFO_SIZE)) {
>+			len = 0;
>+			rc = 0;
>+			goto exit;
>+		}
>+
>+		to_return = AFP_INFO_SIZE - offset;
>+
> 		ai = afpinfo_new(talloc_tos());
> 		if (ai == NULL) {
> 			rc = -1;
>@@ -2636,11 +2647,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 8f63007f508f6f8b6e4e2acb5a40765de3b64624 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>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 <slow@samba.org>
>---
> 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
>

Flags:

vl: review+

Actions: View

Attachments on bug 11363: 11201 | 11224 | 11227