[PATCH v2] Fix bug 10775 - smbd crashes when accessing garbage filenames

Jeremy Allison jra at samba.org
Fri Sep 12 15:40:59 MDT 2014


On Fri, Sep 12, 2014 at 08:57:32AM -0700, Jeremy Allison wrote:
> 
> Please reconsider the attached patch :-).

Actually, this one :-). I realized that
the "can we correctly srvstr_push this
name" check has to be done in the old
DOS 8.3 codepath as well, otherwise
a client could still crash us by
accessing an invalid name via selecting
the CORE protocol and getting a name
listing that way :-).

Even the refactoring we discussed
earlier wouldn't have caught that
one, as it depends on the forced
mangling functions catching bad
conversions - which they currently
don't do :-).

Cheers,

	Jeremy.
-------------- next part --------------
From 5768bf7e896c5bad996faf5e8e133e1b5d8bd29c Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 12 Sep 2014 08:46:06 -0700
Subject: [PATCH 01/11] s3: utils: Don't directly manipulate errno inside
 strupper_m().

Let the internal character conversion routines set it.

Caller code paths don't depend on this (checked by
David Disseldorp ddiss at suse.de).

Bug 10775 - smbd crashes when accessing garbage filenames

https://bugzilla.samba.org/show_bug.cgi?id=10775

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/lib/util_str.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/source3/lib/util_str.c b/source3/lib/util_str.c
index cfc495d..2b0830c 100644
--- a/source3/lib/util_str.c
+++ b/source3/lib/util_str.c
@@ -551,7 +551,6 @@ _PUBLIC_ void strupper_m(char *s)
 bool strupper_m(char *s)
 {
 	size_t len;
-	int errno_save;
 	bool ret = false;
 
 	/* this is quite a common operation, so we want it to be
@@ -570,14 +569,11 @@ bool strupper_m(char *s)
 	/* I assume that lowercased string takes the same number of bytes
 	 * as source string even in multibyte encoding. (VIV) */
 	len = strlen(s) + 1;
-	errno_save = errno;
-	errno = 0;
 	ret = unix_strupper(s,len,s,len);
 	/* Catch mb conversion errors that may not terminate. */
-	if (errno) {
+	if (!ret) {
 		s[len-1] = '\0';
 	}
-	errno = errno_save;
 	return ret;
 }
 
-- 
2.1.0.rc2.206.gedb03e5


From 91286d5d7a96bf5d06f5f0f05b3c039848e949ad Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 25 Aug 2014 16:21:24 -0700
Subject: [PATCH 02/11] s3: smbd: srvstr_push() was changed to never return -1,
 so don't check for that as an error.

Bug 10775 - smbd crashes when accessing garbage filenames

https://bugzilla.samba.org/show_bug.cgi?id=10775

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/srvstr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/smbd/srvstr.c b/source3/smbd/srvstr.c
index c069dd4..648c69f 100644
--- a/source3/smbd/srvstr.c
+++ b/source3/smbd/srvstr.c
@@ -65,7 +65,7 @@ ssize_t message_push_string(uint8 **outbuf, const char *str, int flags)
 	result = srvstr_push((char *)tmp, SVAL(tmp, smb_flg2),
 			     tmp + buf_size, str, grow_size, flags);
 
-	if (result == (size_t)-1) {
+	if (result == 0) {
 		DEBUG(0, ("srvstr_push failed\n"));
 		return -1;
 	}
-- 
2.1.0.rc2.206.gedb03e5


From f414827312ee084d784e40390bf5fcf0990ff445 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 25 Aug 2014 17:05:47 -0700
Subject: [PATCH 03/11] s3: smbd: Ensure types for all variables called 'len'
 used in srvstr_push() are correct.

Bug 10775 - smbd crashes when accessing garbage filenames

https://bugzilla.samba.org/show_bug.cgi?id=10775

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/trans2.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 1e2c02e..70d29f2 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -1594,7 +1594,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 	uint64_t file_size = 0;
 	uint64_t allocation_size = 0;
 	uint64_t file_index = 0;
-	uint32_t len;
+	size_t len = 0;
 	struct timespec mdate_ts, adate_ts, cdate_ts, create_date_ts;
 	time_t mdate = (time_t)0, adate = (time_t)0, create_date = (time_t)0;
 	char *nameptr;
@@ -3077,7 +3077,8 @@ NTSTATUS smbd_do_qfsinfo(struct smbXsrv_connection *xconn,
 			 int *ret_data_len)
 {
 	char *pdata, *end_data;
-	int data_len = 0, len;
+	int data_len = 0;
+	size_t len = 0;
 	const char *vname = volume_label(talloc_tos(), SNUM(conn));
 	int snum = SNUM(conn);
 	const char *fstype = lp_fstype(SNUM(conn));
@@ -3187,9 +3188,9 @@ cBytesSector=%u, cUnitTotal=%u, cUnitAvail=%d\n", (unsigned int)st.st_ex_dev, (u
 				STR_NOALIGN|STR_TERMINATE);
 			SCVAL(pdata,l2_vol_cch,len);
 			data_len = l2_vol_szVolLabel + len;
-			DEBUG(5,("smbd_do_qfsinfo : time = %x, namelen = %d, name = %s\n",
+			DEBUG(5,("smbd_do_qfsinfo : time = %x, namelen = %u, name = %s\n",
 				 (unsigned)convert_timespec_to_time_t(st.st_ex_ctime),
-				 len, vname));
+				 (unsigned)len, vname));
 			break;
 
 		case SMB_QUERY_FS_ATTRIBUTE_INFO:
@@ -4426,6 +4427,7 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 	uint64_t allocation_size = 0;
 	uint64_t file_index = 0;
 	uint32_t access_mask = 0;
+	size_t len = 0;
 
 	if (INFO_LEVEL_IS_UNIX(info_level) && !lp_unix_extensions()) {
 		return NT_STATUS_INVALID_LEVEL;
@@ -4727,7 +4729,6 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 		case SMB_QUERY_FILE_ALT_NAME_INFO:
 		case SMB_FILE_ALTERNATE_NAME_INFORMATION:
 		{
-			int len;
 			char mangled_name[13];
 			DEBUG(10,("smbd_do_qfilepathinfo: SMB_FILE_ALTERNATE_NAME_INFORMATION\n"));
 			if (!name_to_8_3(base_name,mangled_name,
@@ -4746,7 +4747,6 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 
 		case SMB_QUERY_FILE_NAME_INFO:
 		{
-			int len;
 			/*
 			  this must be *exactly* right for ACLs on mapped drives to work
 			 */
@@ -4777,7 +4777,6 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 		case SMB_QUERY_FILE_ALL_INFO:
 		case SMB_FILE_ALL_INFORMATION:
 		{
-			int len;
 			unsigned int ea_size =
 			    estimate_ea_size(conn, fsp, smb_fname);
 			DEBUG(10,("smbd_do_qfilepathinfo: SMB_FILE_ALL_INFORMATION\n"));
@@ -4810,7 +4809,6 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 
 		case 0xFF12:/*SMB2_FILE_ALL_INFORMATION*/
 		{
-			int len;
 			unsigned int ea_size =
 			    estimate_ea_size(conn, fsp, smb_fname);
 			DEBUG(10,("smbd_do_qfilepathinfo: SMB2_FILE_ALL_INFORMATION\n"));
@@ -5010,7 +5008,7 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 
 		case SMB_QUERY_FILE_UNIX_LINK:
 			{
-				int len;
+				int link_len = 0;
 				char *buffer = talloc_array(mem_ctx, char, PATH_MAX+1);
 
 				if (!buffer) {
@@ -5025,13 +5023,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 #else
 				return NT_STATUS_DOS(ERRDOS, ERRbadlink);
 #endif
-				len = SMB_VFS_READLINK(conn,
+				link_len = SMB_VFS_READLINK(conn,
 						       smb_fname->base_name,
 						       buffer, PATH_MAX);
-				if (len == -1) {
+				if (link_len == -1) {
 					return map_nt_error_from_unix(errno);
 				}
-				buffer[len] = 0;
+				buffer[link_len] = 0;
 				len = srvstr_push(dstart, flags2,
 						  pdata, buffer,
 						  PTR_DIFF(dend, pdata),
-- 
2.1.0.rc2.206.gedb03e5


From c36f0b3fc9007a4f21595ea0ac474d3246a7c79c Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 25 Aug 2014 17:11:58 -0700
Subject: [PATCH 04/11] s3: smbd: Change the function signature of
 srvstr_push() from returning a length to returning an NTSTATUS with a length
 param.

srvstr_push_fn() now returns an NTSTATUS reporting any
string conversion failure.

We need to get serious about returning character set conversion errors
inside smbd.

Bug 10775 - smbd crashes when accessing garbage filenames

https://bugzilla.samba.org/show_bug.cgi?id=10775

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 lib/util/samba_util.h         |   1 +
 lib/util/string_wrappers.h    |   8 +-
 source3/include/safe_string.h |   1 +
 source3/modules/vfs_default.c |  10 ++-
 source3/smbd/lanman.c         |   9 ++-
 source3/smbd/proto.h          |   4 +-
 source3/smbd/reply.c          |  30 ++++++--
 source3/smbd/srvstr.c         |  57 ++++++++++++--
 source3/smbd/trans2.c         | 174 ++++++++++++++++++++++++++++++------------
 9 files changed, 221 insertions(+), 73 deletions(-)

diff --git a/lib/util/samba_util.h b/lib/util/samba_util.h
index f1f4c2d..528d373 100644
--- a/lib/util/samba_util.h
+++ b/lib/util/samba_util.h
@@ -63,6 +63,7 @@ do { \
 
 #include "lib/util/memory.h"
 
+#include "../libcli/util/ntstatus.h"
 #include "lib/util/string_wrappers.h"
 
 /**
diff --git a/lib/util/string_wrappers.h b/lib/util/string_wrappers.h
index fcc088c..1feea8c 100644
--- a/lib/util/string_wrappers.h
+++ b/lib/util/string_wrappers.h
@@ -57,6 +57,8 @@ char * __unsafe_string_function_usage_here__(void);
 
 size_t __unsafe_string_function_usage_here_size_t__(void);
 
+NTSTATUS __unsafe_string_function_usage_here_NTSTATUS__(void);
+
 #define CHECK_STRING_SIZE(d, len) (sizeof(d) != (len) && sizeof(d) != sizeof(char *))
 
 /* if the compiler will optimize out function calls, then use this to tell if we are
@@ -68,10 +70,10 @@ size_t __unsafe_string_function_usage_here_size_t__(void);
     ? __unsafe_string_function_usage_here_size_t__() \
     : push_string_check_fn(dest, src, dest_len, flags))
 
-#define srvstr_push(base_ptr, smb_flags2, dest, src, dest_len, flags) \
+#define srvstr_push(base_ptr, smb_flags2, dest, src, dest_len, flags, ret_len) \
     (CHECK_STRING_SIZE(dest, dest_len) \
-    ? __unsafe_string_function_usage_here_size_t__() \
-    : srvstr_push_fn(base_ptr, smb_flags2, dest, src, dest_len, flags))
+    ? __unsafe_string_function_usage_here_NTSTATUS__() \
+    : srvstr_push_fn(base_ptr, smb_flags2, dest, src, dest_len, flags, ret_len))
 
 /* This allows the developer to choose to check the arguments to
    strlcpy.  if the compiler will optimize out function calls, then
diff --git a/source3/include/safe_string.h b/source3/include/safe_string.h
index 03878b4..e77017c 100644
--- a/source3/include/safe_string.h
+++ b/source3/include/safe_string.h
@@ -62,6 +62,7 @@
 
 #endif /* !_SPLINT_ */
 
+#include "../libcli/util/ntstatus.h"
 #include "lib/util/string_wrappers.h"
 
 #endif
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 3430cd0..3a3943b 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -1178,10 +1178,16 @@ static NTSTATUS vfswrap_fsctl(struct vfs_handle_struct *handle,
 			  shadow_data->num_volumes, fsp_str_dbg(fsp)));
 		if (labels && shadow_data->labels) {
 			for (i=0; i<shadow_data->num_volumes; i++) {
-				srvstr_push(cur_pdata, req_flags,
+				size_t len = 0;
+				status = srvstr_push(cur_pdata, req_flags,
 					    cur_pdata, shadow_data->labels[i],
 					    2 * sizeof(SHADOW_COPY_LABEL),
-					    STR_UNICODE|STR_TERMINATE);
+					    STR_UNICODE|STR_TERMINATE, &len);
+				if (!NT_STATUS_IS_OK(status)) {
+					TALLOC_FREE(*out_data);
+					TALLOC_FREE(shadow_data);
+					return status;
+				}
 				cur_pdata += 2 * sizeof(SHADOW_COPY_LABEL);
 				DEBUGADD(10,("Label[%u]: '%s'\n",i,shadow_data->labels[i]));
 			}
diff --git a/source3/smbd/lanman.c b/source3/smbd/lanman.c
index b7c74e9..ac4873d 100644
--- a/source3/smbd/lanman.c
+++ b/source3/smbd/lanman.c
@@ -3655,8 +3655,13 @@ static bool api_RNetServerGetInfo(struct smbd_server_connection *sconn,
 	}
 
 	if (uLevel != 20) {
-		srvstr_push(NULL, 0, p, info.info101->server_name, 16,
-			STR_ASCII|STR_UPPER|STR_TERMINATE);
+		size_t len = 0;
+		status = srvstr_push(NULL, 0, p, info.info101->server_name, 16,
+			STR_ASCII|STR_UPPER|STR_TERMINATE, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			errcode = W_ERROR_V(ntstatus_to_werror(status));
+			goto out;
+		}
   	}
 	p += 16;
 	if (uLevel > 0) {
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index f2b5888..3a5ebcc 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -1011,8 +1011,8 @@ bool is_share_read_only_for_token(const char *username,
 
 /* The following definitions come from smbd/srvstr.c  */
 
-size_t srvstr_push_fn(const char *base_ptr, uint16 smb_flags2, void *dest,
-		      const char *src, int dest_len, int flags);
+NTSTATUS srvstr_push_fn(const char *base_ptr, uint16 smb_flags2, void *dest,
+		      const char *src, int dest_len, int flags, size_t *ret_len);
 ssize_t message_push_string(uint8 **outbuf, const char *str, int flags);
 
 /* The following definitions come from smbd/statcache.c  */
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 2422ad3..3c22bbb 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -1099,6 +1099,8 @@ void reply_ioctl(struct smb_request *req)
 	switch (ioctl_code) {
 		case IOCTL_QUERY_JOB_INFO:		    
 		{
+			NTSTATUS status;
+			size_t len = 0;
 			files_struct *fsp = file_fsp(
 				req, SVAL(req->vwv+0, 0));
 			if (!fsp) {
@@ -1109,15 +1111,25 @@ void reply_ioctl(struct smb_request *req)
 			/* Job number */
 			SSVAL(p, 0, print_spool_rap_jobid(fsp->print_file));
 
-			srvstr_push((char *)req->outbuf, req->flags2, p+2,
+			status = srvstr_push((char *)req->outbuf, req->flags2, p+2,
 				    lp_netbios_name(), 15,
-				    STR_TERMINATE|STR_ASCII);
+				    STR_TERMINATE|STR_ASCII, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				reply_nterror(req, status);
+				END_PROFILE(SMBioctl);
+				return;
+			}
 			if (conn) {
-				srvstr_push((char *)req->outbuf, req->flags2,
+				status = srvstr_push((char *)req->outbuf, req->flags2,
 					    p+18,
 					    lp_servicename(talloc_tos(),
 							   SNUM(conn)),
-					    13, STR_TERMINATE|STR_ASCII);
+					    13, STR_TERMINATE|STR_ASCII, &len);
+				if (!NT_STATUS_IS_OK(status)) {
+					reply_nterror(req, status);
+					END_PROFILE(SMBioctl);
+					return;
+				}
 			} else {
 				memset(p+18, 0, 13);
 			}
@@ -5791,6 +5803,7 @@ void reply_printqueue(struct smb_request *req)
 			char *p = blob;
 			time_t qtime = spoolss_Time_to_time_t(&info[i].info2.submitted);
 			int qstatus;
+			size_t len = 0;
 			uint16_t qrapjobid = pjobid_to_rap(sharename,
 							info[i].info2.job_id);
 
@@ -5805,9 +5818,12 @@ void reply_printqueue(struct smb_request *req)
 			SSVAL(p, 5, qrapjobid);
 			SIVAL(p, 7, info[i].info2.size);
 			SCVAL(p, 11, 0);
-			srvstr_push(blob, req->flags2, p+12,
-				    info[i].info2.notify_name, 16, STR_ASCII);
-
+			status = srvstr_push(blob, req->flags2, p+12,
+				    info[i].info2.notify_name, 16, STR_ASCII, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				reply_nterror(req, status);
+				goto out;
+			}
 			if (message_push_blob(
 				    &req->outbuf,
 				    data_blob_const(
diff --git a/source3/smbd/srvstr.c b/source3/smbd/srvstr.c
index 648c69f..e6a8541 100644
--- a/source3/smbd/srvstr.c
+++ b/source3/smbd/srvstr.c
@@ -24,16 +24,56 @@
 
 /* Make sure we can't write a string past the end of the buffer */
 
-size_t srvstr_push_fn(const char *base_ptr, uint16 smb_flags2, void *dest,
-		      const char *src, int dest_len, int flags)
+NTSTATUS srvstr_push_fn(const char *base_ptr, uint16 smb_flags2, void *dest,
+		      const char *src, int dest_len, int flags, size_t *ret_len)
 {
+	size_t len;
+	int saved_errno;
+	NTSTATUS status;
+
 	if (dest_len < 0) {
-		return 0;
+		return NT_STATUS_INVALID_PARAMETER;
 	}
 
+	saved_errno = errno;
+	errno = 0;
+
 	/* 'normal' push into size-specified buffer */
-	return push_string_base(base_ptr, smb_flags2, dest, src,
+	len = push_string_base(base_ptr, smb_flags2, dest, src,
 				dest_len, flags);
+
+	if (errno != 0) {
+		/*
+		 * Special case E2BIG, EILSEQ, EINVAL
+		 * as they mean conversion errors here,
+		 * but we don't generically map them as
+		 * they can mean different things in
+		 * generic filesystem calls (such as
+		 * read xattrs).
+		 */
+		if (errno == E2BIG || errno == EILSEQ || errno == EINVAL) {
+			status = NT_STATUS_ILLEGAL_CHARACTER;
+		} else {
+			status = map_nt_error_from_unix_common(errno);
+			/*
+			 * Paranoia - Filter out STATUS_MORE_ENTRIES.
+			 * I don't think we can get this but it has a
+			 * specific meaning to the client.
+			 */
+			if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
+				status = NT_STATUS_UNSUCCESSFUL;
+			}
+		}
+		DEBUG(10,("character conversion failure "
+			"on string (%s) (%s)\n",
+			src, strerror(errno)));
+	} else {
+		/* Success - restore untouched errno. */
+		errno = saved_errno;
+		*ret_len = len;
+		status = NT_STATUS_OK;
+	}
+	return status;
 }
 
 /*******************************************************************
@@ -45,8 +85,9 @@ ssize_t message_push_string(uint8 **outbuf, const char *str, int flags)
 {
 	size_t buf_size = smb_len(*outbuf) + 4;
 	size_t grow_size;
-	size_t result;
+	size_t result = 0;
 	uint8 *tmp;
+	NTSTATUS status;
 
 	/*
 	 * We need to over-allocate, now knowing what srvstr_push will
@@ -62,10 +103,10 @@ ssize_t message_push_string(uint8 **outbuf, const char *str, int flags)
 		return -1;
 	}
 
-	result = srvstr_push((char *)tmp, SVAL(tmp, smb_flg2),
-			     tmp + buf_size, str, grow_size, flags);
+	status = srvstr_push((char *)tmp, SVAL(tmp, smb_flg2),
+			     tmp + buf_size, str, grow_size, flags, &result);
 
-	if (result == 0) {
+	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(0, ("srvstr_push failed\n"));
 		return -1;
 	}
diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 70d29f2..bdecc60 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -1602,6 +1602,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 	bool was_8_3;
 	int off;
 	int pad = 0;
+	NTSTATUS status;
 
 	*out_of_space = false;
 
@@ -1684,9 +1685,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		if (flags2 & FLAGS2_UNICODE_STRINGS) {
 			p += ucs2_align(base_data, p, 0);
 		}
-		len = srvstr_push(base_data, flags2, p,
+		status = srvstr_push(base_data, flags2, p,
 				  fname, PTR_DIFF(end_data, p),
-				  STR_TERMINATE);
+				  STR_TERMINATE, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		if (flags2 & FLAGS2_UNICODE_STRINGS) {
 			if (len > 2) {
 				SCVAL(nameptr, -1, len - 2);
@@ -1722,9 +1726,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		}
 		p += 27;
 		nameptr = p - 1;
-		len = srvstr_push(base_data, flags2,
+		status = srvstr_push(base_data, flags2,
 				  p, fname, PTR_DIFF(end_data, p),
-				  STR_TERMINATE | STR_NOALIGN);
+				  STR_TERMINATE | STR_NOALIGN, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		if (flags2 & FLAGS2_UNICODE_STRINGS) {
 			if (len > 2) {
 				len -= 2;
@@ -1747,7 +1754,6 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 	{
 		struct ea_list *file_list = NULL;
 		size_t ea_len = 0;
-		NTSTATUS status;
 
 		DEBUG(10,("smbd_marshall_dir_entry: SMB_FIND_EA_LIST\n"));
 		if (!name_list) {
@@ -1787,9 +1793,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		/* Push the ea_data followed by the name. */
 		p += fill_ea_buffer(ctx, p, space_remaining, conn, name_list);
 		nameptr = p;
-		len = srvstr_push(base_data, flags2,
+		status = srvstr_push(base_data, flags2,
 				  p + 1, fname, PTR_DIFF(end_data, p+1),
-				  STR_TERMINATE | STR_NOALIGN);
+				  STR_TERMINATE | STR_NOALIGN, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		if (flags2 & FLAGS2_UNICODE_STRINGS) {
 			if (len > 2) {
 				len -= 2;
@@ -1842,9 +1851,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				memset(mangled_name,'\0',12);
 			}
 			mangled_name[12] = 0;
-			len = srvstr_push(base_data, flags2,
+			status = srvstr_push(base_data, flags2,
 					  p+2, mangled_name, 24,
-					  STR_UPPER|STR_UNICODE);
+					  STR_UPPER|STR_UNICODE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return false;
+			}
 			if (len < 24) {
 				memset(p + 2 + len,'\0',24 - len);
 			}
@@ -1853,9 +1865,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 			memset(p,'\0',26);
 		}
 		p += 2 + 24;
-		len = srvstr_push(base_data, flags2, p,
+		status = srvstr_push(base_data, flags2, p,
 				  fname, PTR_DIFF(end_data, p),
-				  STR_TERMINATE_ASCII);
+				  STR_TERMINATE_ASCII, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		SIVAL(q,0,len);
 		p += len;
 
@@ -1889,9 +1904,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		SOFF_T(p,0,file_size); p += 8;
 		SOFF_T(p,0,allocation_size); p += 8;
 		SIVAL(p,0,mode); p += 4;
-		len = srvstr_push(base_data, flags2,
+		status = srvstr_push(base_data, flags2,
 				  p + 4, fname, PTR_DIFF(end_data, p+4),
-				  STR_TERMINATE_ASCII);
+				  STR_TERMINATE_ASCII, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		SIVAL(p,0,len);
 		p += 4 + len;
 
@@ -1932,9 +1950,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 			SIVAL(p,0,ea_size); /* Extended attributes */
 			p +=4;
 		}
-		len = srvstr_push(base_data, flags2, p,
+		status = srvstr_push(base_data, flags2, p,
 				  fname, PTR_DIFF(end_data, p),
-				  STR_TERMINATE_ASCII);
+				  STR_TERMINATE_ASCII, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		SIVAL(q, 0, len);
 		p += len;
 
@@ -1964,9 +1985,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		p += 4;
 		/* this must *not* be null terminated or w2k gets in a loop trying to set an
 		   acl on a dir (tridge) */
-		len = srvstr_push(base_data, flags2, p,
+		status = srvstr_push(base_data, flags2, p,
 				  fname, PTR_DIFF(end_data, p),
-				  STR_TERMINATE_ASCII);
+				  STR_TERMINATE_ASCII, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		SIVAL(p, -4, len);
 		p += len;
 
@@ -2011,9 +2035,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		p += 4;
 		SIVAL(p,0,0); p += 4; /* Unknown - reserved ? */
 		SBVAL(p,0,file_index); p += 8;
-		len = srvstr_push(base_data, flags2, p,
+		status = srvstr_push(base_data, flags2, p,
 				  fname, PTR_DIFF(end_data, p),
-				  STR_TERMINATE_ASCII);
+				  STR_TERMINATE_ASCII, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		SIVAL(q, 0, len);
 		p += len;
 
@@ -2069,9 +2096,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				memset(mangled_name,'\0',12);
 			}
 			mangled_name[12] = 0;
-			len = srvstr_push(base_data, flags2,
+			status = srvstr_push(base_data, flags2,
 					  p+2, mangled_name, 24,
-					  STR_UPPER|STR_UNICODE);
+					  STR_UPPER|STR_UNICODE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return false;
+			}
 			SSVAL(p, 0, len);
 			if (len < 24) {
 				memset(p + 2 + len,'\0',24 - len);
@@ -2083,9 +2113,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		p += 26;
 		SSVAL(p,0,0); p += 2; /* Reserved ? */
 		SBVAL(p,0,file_index); p += 8;
-		len = srvstr_push(base_data, flags2, p,
+		status = srvstr_push(base_data, flags2, p,
 				  fname, PTR_DIFF(end_data, p),
-				  STR_TERMINATE_ASCII);
+				  STR_TERMINATE_ASCII, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		SIVAL(q,0,len);
 		p += len;
 
@@ -2121,17 +2154,23 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 			DEBUG(10,("smbd_marshall_dir_entry: SMB_FIND_FILE_UNIX\n"));
 			p = store_file_unix_basic(conn, p,
 						NULL, &smb_fname->st);
-			len = srvstr_push(base_data, flags2, p,
+			status = srvstr_push(base_data, flags2, p,
 					  fname, PTR_DIFF(end_data, p),
-					  STR_TERMINATE);
+					  STR_TERMINATE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return false;
+			}
 		} else {
 			DEBUG(10,("smbd_marshall_dir_entry: SMB_FIND_FILE_UNIX_INFO2\n"));
 			p = store_file_unix_basic_info2(conn, p,
 						NULL, &smb_fname->st);
 			nameptr = p;
 			p += 4;
-			len = srvstr_push(base_data, flags2, p, fname,
-					  PTR_DIFF(end_data, p), 0);
+			status = srvstr_push(base_data, flags2, p, fname,
+					  PTR_DIFF(end_data, p), 0, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return false;
+			}
 			SIVAL(nameptr, 0, len);
 		}
 
@@ -3181,11 +3220,14 @@ cBytesSector=%u, cUnitTotal=%u, cUnitAvail=%d\n", (unsigned int)st.st_ex_dev, (u
 			 * this call so try fixing this by adding a terminating null to
 			 * the pushed string. The change here was adding the STR_TERMINATE. JRA.
 			 */
-			len = srvstr_push(
+			status = srvstr_push(
 				pdata, flags2,
 				pdata+l2_vol_szVolLabel, vname,
 				PTR_DIFF(end_data, pdata+l2_vol_szVolLabel),
-				STR_NOALIGN|STR_TERMINATE);
+				STR_NOALIGN|STR_TERMINATE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
+			}
 			SCVAL(pdata,l2_vol_cch,len);
 			data_len = l2_vol_szVolLabel + len;
 			DEBUG(5,("smbd_do_qfsinfo : time = %x, namelen = %u, name = %s\n",
@@ -3218,9 +3260,12 @@ cBytesSector=%u, cUnitTotal=%u, cUnitAvail=%d\n", (unsigned int)st.st_ex_dev, (u
 			SIVAL(pdata,4,255); /* Max filename component length */
 			/* NOTE! the fstype must *not* be null terminated or win98 won't recognise it
 				and will think we can't do long filenames */
-			len = srvstr_push(pdata, flags2, pdata+12, fstype,
+			status = srvstr_push(pdata, flags2, pdata+12, fstype,
 					  PTR_DIFF(end_data, pdata+12),
-					  STR_UNICODE);
+					  STR_UNICODE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
+			}
 			SIVAL(pdata,8,len);
 			data_len = 12 + len;
 			if (max_data_bytes >= 16 && data_len > max_data_bytes) {
@@ -3234,8 +3279,11 @@ cBytesSector=%u, cUnitTotal=%u, cUnitAvail=%d\n", (unsigned int)st.st_ex_dev, (u
 
 		case SMB_QUERY_FS_LABEL_INFO:
 		case SMB_FS_LABEL_INFORMATION:
-			len = srvstr_push(pdata, flags2, pdata+4, vname,
-					  PTR_DIFF(end_data, pdata+4), 0);
+			status = srvstr_push(pdata, flags2, pdata+4, vname,
+					  PTR_DIFF(end_data, pdata+4), 0, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
+			}
 			data_len = 4 + len;
 			SIVAL(pdata,0,len);
 			break;
@@ -3251,9 +3299,12 @@ cBytesSector=%u, cUnitTotal=%u, cUnitAvail=%d\n", (unsigned int)st.st_ex_dev, (u
 				(str_checksum(get_local_machine_name())<<16));
 
 			/* Max label len is 32 characters. */
-			len = srvstr_push(pdata, flags2, pdata+18, vname,
+			status = srvstr_push(pdata, flags2, pdata+18, vname,
 					  PTR_DIFF(end_data, pdata+18),
-					  STR_UNICODE);
+					  STR_UNICODE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
+			}
 			SIVAL(pdata,12,len);
 			data_len = 18+len;
 
@@ -4735,10 +4786,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 						True,conn->params)) {
 				return NT_STATUS_NO_MEMORY;
 			}
-			len = srvstr_push(dstart, flags2,
+			status = srvstr_push(dstart, flags2,
 					  pdata+4, mangled_name,
 					  PTR_DIFF(dend, pdata+4),
-					  STR_UNICODE);
+					  STR_UNICODE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
+			}
 			data_size = 4 + len;
 			SIVAL(pdata,0,len);
 			*fixed_portion = 8;
@@ -4750,10 +4804,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 			/*
 			  this must be *exactly* right for ACLs on mapped drives to work
 			 */
-			len = srvstr_push(dstart, flags2,
+			status = srvstr_push(dstart, flags2,
 					  pdata+4, dos_fname,
 					  PTR_DIFF(dend, pdata+4),
-					  STR_UNICODE);
+					  STR_UNICODE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
+			}
 			DEBUG(10,("smbd_do_qfilepathinfo: SMB_QUERY_FILE_NAME_INFO\n"));
 			data_size = 4 + len;
 			SIVAL(pdata,0,len);
@@ -4796,10 +4853,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 			pdata += 24;
 			SIVAL(pdata,0,ea_size);
 			pdata += 4; /* EA info */
-			len = srvstr_push(dstart, flags2,
+			status = srvstr_push(dstart, flags2,
 					  pdata+4, dos_fname,
 					  PTR_DIFF(dend, pdata+4),
-					  STR_UNICODE);
+					  STR_UNICODE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
+			}
 			SIVAL(pdata,0,len);
 			pdata += 4 + len;
 			data_size = PTR_DIFF(pdata,(*ppdata));
@@ -4833,10 +4893,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 
 			pdata += 0x60;
 
-			len = srvstr_push(dstart, flags2,
+			status = srvstr_push(dstart, flags2,
 					  pdata+4, dos_fname,
 					  PTR_DIFF(dend, pdata+4),
-					  STR_UNICODE);
+					  STR_UNICODE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
+			}
 			SIVAL(pdata,0,len);
 			pdata += 4 + len;
 			data_size = PTR_DIFF(pdata,(*ppdata));
@@ -5030,10 +5093,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 					return map_nt_error_from_unix(errno);
 				}
 				buffer[link_len] = 0;
-				len = srvstr_push(dstart, flags2,
+				status = srvstr_push(dstart, flags2,
 						  pdata, buffer,
 						  PTR_DIFF(dend, pdata),
-						  STR_TERMINATE);
+						  STR_TERMINATE, &len);
+				if (!NT_STATUS_IS_OK(status)) {
+					return status;
+				}
 				pdata += len;
 				data_size = PTR_DIFF(pdata,(*ppdata));
 
@@ -8602,6 +8668,8 @@ static void call_trans2ioctl(connection_struct *conn,
 {
 	char *pdata = *ppdata;
 	files_struct *fsp = file_fsp(req, SVAL(req->vwv+15, 0));
+	NTSTATUS status;
+	size_t len = 0;
 
 	/* check for an invalid fid before proceeding */
 
@@ -8625,12 +8693,20 @@ static void call_trans2ioctl(connection_struct *conn,
 		/* Job number */
 		SSVAL(pdata, 0, print_spool_rap_jobid(fsp->print_file));
 
-		srvstr_push(pdata, req->flags2, pdata + 2,
+		status = srvstr_push(pdata, req->flags2, pdata + 2,
 			    lp_netbios_name(), 15,
-			    STR_ASCII|STR_TERMINATE); /* Our NetBIOS name */
-		srvstr_push(pdata, req->flags2, pdata+18,
+			    STR_ASCII|STR_TERMINATE, &len); /* Our NetBIOS name */
+		if (!NT_STATUS_IS_OK(status)) {
+			reply_nterror(req, status);
+			return;
+		}
+		status = srvstr_push(pdata, req->flags2, pdata+18,
 			    lp_servicename(talloc_tos(), SNUM(conn)), 13,
-			    STR_ASCII|STR_TERMINATE); /* Service name */
+			    STR_ASCII|STR_TERMINATE, &len); /* Service name */
+		if (!NT_STATUS_IS_OK(status)) {
+			reply_nterror(req, status);
+			return;
+		}
 		send_trans2_replies(conn, req, NT_STATUS_OK, *pparams, 0, *ppdata, 32,
 				    max_data_bytes);
 		return;
-- 
2.1.0.rc2.206.gedb03e5


From 6763f71554db99075db3bc247af26f077ffb5294 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 26 Aug 2014 11:36:41 -0700
Subject: [PATCH 05/11] s3: smbd: Change smbd_marshall_dir_entry() to return an
 NTSTATUS. Returns STATUS_MORE_ENTRIES on out of space.

Bug 10775 - smbd crashes when accessing garbage filenames

https://bugzilla.samba.org/show_bug.cgi?id=10775

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/trans2.c | 47 ++++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index bdecc60..1a145c7 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -1570,7 +1570,7 @@ static bool smbd_dirptr_lanman2_mode_fn(TALLOC_CTX *ctx,
 	return true;
 }
 
-static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
+static NTSTATUS smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				    connection_struct *conn,
 				    uint16_t flags2,
 				    uint32_t info_level,
@@ -1647,7 +1647,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 			"for padding (wanted %u, had %d)\n",
 			(unsigned int)pad,
 			space_remaining ));
-		return false; /* Not finished - just out of space */
+		return STATUS_MORE_ENTRIES; /* Not finished - just out of space */
 	}
 
 	off += pad;
@@ -1689,7 +1689,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  fname, PTR_DIFF(end_data, p),
 				  STR_TERMINATE, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		if (flags2 & FLAGS2_UNICODE_STRINGS) {
 			if (len > 2) {
@@ -1730,7 +1730,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  p, fname, PTR_DIFF(end_data, p),
 				  STR_TERMINATE | STR_NOALIGN, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		if (flags2 & FLAGS2_UNICODE_STRINGS) {
 			if (len > 2) {
@@ -1757,7 +1757,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 
 		DEBUG(10,("smbd_marshall_dir_entry: SMB_FIND_EA_LIST\n"));
 		if (!name_list) {
-			return false;
+			return NT_STATUS_INVALID_PARAMETER;
 		}
 		if (requires_resume_key) {
 			SIVAL(p,0,reskey);
@@ -1787,7 +1787,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				"(wanted %u, had %d)\n",
 				(unsigned int)PTR_DIFF(p + 255 + ea_len,pdata),
 				space_remaining ));
-			return False; /* Not finished - just out of space */
+			return STATUS_MORE_ENTRIES; /* Not finished - just out of space */
 		}
 
 		/* Push the ea_data followed by the name. */
@@ -1797,7 +1797,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  p + 1, fname, PTR_DIFF(end_data, p+1),
 				  STR_TERMINATE | STR_NOALIGN, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		if (flags2 & FLAGS2_UNICODE_STRINGS) {
 			if (len > 2) {
@@ -1855,7 +1855,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 					  p+2, mangled_name, 24,
 					  STR_UPPER|STR_UNICODE, &len);
 			if (!NT_STATUS_IS_OK(status)) {
-				return false;
+				return status;
 			}
 			if (len < 24) {
 				memset(p + 2 + len,'\0',24 - len);
@@ -1869,7 +1869,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  fname, PTR_DIFF(end_data, p),
 				  STR_TERMINATE_ASCII, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		SIVAL(q,0,len);
 		p += len;
@@ -1908,7 +1908,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  p + 4, fname, PTR_DIFF(end_data, p+4),
 				  STR_TERMINATE_ASCII, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		SIVAL(p,0,len);
 		p += 4 + len;
@@ -1954,7 +1954,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  fname, PTR_DIFF(end_data, p),
 				  STR_TERMINATE_ASCII, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		SIVAL(q, 0, len);
 		p += len;
@@ -1989,7 +1989,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  fname, PTR_DIFF(end_data, p),
 				  STR_TERMINATE_ASCII, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		SIVAL(p, -4, len);
 		p += len;
@@ -2039,7 +2039,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  fname, PTR_DIFF(end_data, p),
 				  STR_TERMINATE_ASCII, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		SIVAL(q, 0, len);
 		p += len;
@@ -2100,7 +2100,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 					  p+2, mangled_name, 24,
 					  STR_UPPER|STR_UNICODE, &len);
 			if (!NT_STATUS_IS_OK(status)) {
-				return false;
+				return status;
 			}
 			SSVAL(p, 0, len);
 			if (len < 24) {
@@ -2117,7 +2117,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  fname, PTR_DIFF(end_data, p),
 				  STR_TERMINATE_ASCII, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		SIVAL(q,0,len);
 		p += len;
@@ -2158,7 +2158,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 					  fname, PTR_DIFF(end_data, p),
 					  STR_TERMINATE, &len);
 			if (!NT_STATUS_IS_OK(status)) {
-				return false;
+				return status;
 			}
 		} else {
 			DEBUG(10,("smbd_marshall_dir_entry: SMB_FIND_FILE_UNIX_INFO2\n"));
@@ -2169,7 +2169,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 			status = srvstr_push(base_data, flags2, p, fname,
 					  PTR_DIFF(end_data, p), 0, &len);
 			if (!NT_STATUS_IS_OK(status)) {
-				return false;
+				return status;
 			}
 			SIVAL(nameptr, 0, len);
 		}
@@ -2198,7 +2198,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		break;
 
 	default:
-		return false;
+		return NT_STATUS_INVALID_LEVEL;
 	}
 
 	if (PTR_DIFF(p,pdata) > space_remaining) {
@@ -2207,7 +2207,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 			"(wanted %u, had %d)\n",
 			(unsigned int)PTR_DIFF(p,pdata),
 			space_remaining ));
-		return false; /* Not finished - just out of space */
+		return STATUS_MORE_ENTRIES; /* Not finished - just out of space */
 	}
 
 	/* Setup the last entry pointer, as an offset from base_data */
@@ -2215,7 +2215,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 	/* Advance the data pointer to the next slot */
 	*ppdata = p;
 
-	return true;
+	return NT_STATUS_OK;
 }
 
 bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
@@ -2248,6 +2248,7 @@ bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 	struct smbd_dirptr_lanman2_state state;
 	bool ok;
 	uint64_t last_entry_off = 0;
+	NTSTATUS status;
 
 	ZERO_STRUCT(state);
 	state.conn = conn;
@@ -2289,7 +2290,7 @@ bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 
 	*got_exact_match = state.got_exact_match;
 
-	ok = smbd_marshall_dir_entry(ctx,
+	status = smbd_marshall_dir_entry(ctx,
 				     conn,
 				     flags2,
 				     info_level,
@@ -2309,11 +2310,11 @@ bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 				     &last_entry_off);
 	TALLOC_FREE(fname);
 	TALLOC_FREE(smb_fname);
-	if (*out_of_space) {
+	if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
 		dptr_SeekDir(dirptr, prev_dirpos);
 		return false;
 	}
-	if (!ok) {
+	if (!NT_STATUS_IS_OK(status)) {
 		return false;
 	}
 
-- 
2.1.0.rc2.206.gedb03e5


From 46b589a771a4900edf7a7e31b7ff6cce842668ee Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 26 Aug 2014 11:40:19 -0700
Subject: [PATCH 06/11] s3: smbd: smbd_marshall_dir_entry() no longer needs
 explicit 'out_of_space' parameter.

Handle this in the caller when it returns STATUS_MORE_ENTRIES.

Bug 10775 - smbd crashes when accessing garbage filenames

https://bugzilla.samba.org/show_bug.cgi?id=10775

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/trans2.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 1a145c7..b950820 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -1586,7 +1586,6 @@ static NTSTATUS smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				    char *base_data,
 				    char **ppdata,
 				    char *end_data,
-				    bool *out_of_space,
 				    uint64_t *last_entry_off)
 {
 	char *p, *q, *pdata = *ppdata;
@@ -1604,8 +1603,6 @@ static NTSTATUS smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 	int pad = 0;
 	NTSTATUS status;
 
-	*out_of_space = false;
-
 	ZERO_STRUCT(mdate_ts);
 	ZERO_STRUCT(adate_ts);
 	ZERO_STRUCT(create_date_ts);
@@ -1642,7 +1639,6 @@ static NTSTATUS smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 	pad -= off;
 
 	if (pad && pad > space_remaining) {
-		*out_of_space = true;
 		DEBUG(9,("smbd_marshall_dir_entry: out of space "
 			"for padding (wanted %u, had %d)\n",
 			(unsigned int)pad,
@@ -1782,7 +1778,6 @@ static NTSTATUS smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		/* We need to determine if this entry will fit in the space available. */
 		/* Max string size is 255 bytes. */
 		if (PTR_DIFF(p + 255 + ea_len,pdata) > space_remaining) {
-			*out_of_space = true;
 			DEBUG(9,("smbd_marshall_dir_entry: out of space "
 				"(wanted %u, had %d)\n",
 				(unsigned int)PTR_DIFF(p + 255 + ea_len,pdata),
@@ -2202,7 +2197,6 @@ static NTSTATUS smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 	}
 
 	if (PTR_DIFF(p,pdata) > space_remaining) {
-		*out_of_space = true;
 		DEBUG(9,("smbd_marshall_dir_entry: out of space "
 			"(wanted %u, had %d)\n",
 			(unsigned int)PTR_DIFF(p,pdata),
@@ -2306,11 +2300,11 @@ bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 				     base_data,
 				     ppdata,
 				     end_data,
-				     out_of_space,
 				     &last_entry_off);
 	TALLOC_FREE(fname);
 	TALLOC_FREE(smb_fname);
 	if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
+		*out_of_space = true;
 		dptr_SeekDir(dirptr, prev_dirpos);
 		return false;
 	}
-- 
2.1.0.rc2.206.gedb03e5


From 7c31c2e41db62dce71d7ab725987cc67dea57c00 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 26 Aug 2014 14:49:37 -0700
Subject: [PATCH 07/11] s3: smbd: SMB2 - change smbd_dirptr_lanman2_entry() to
 return an NTSTATUS.

Handle the errors correctly at the top level inside the SMB2 server.

Bug 10775 - smbd crashes when accessing garbage filenames

https://bugzilla.samba.org/show_bug.cgi?id=10775

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/globals.h   |  2 +-
 source3/smbd/smb2_find.c | 15 ++++++++++-----
 source3/smbd/trans2.c    | 14 ++++++++------
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index f78ce45..20ab75d 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -185,7 +185,7 @@ bool smbd_dirptr_get_entry(TALLOC_CTX *ctx,
 			   uint32_t *_mode,
 			   long *_prev_offset);
 
-bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
+NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 			       connection_struct *conn,
 			       struct dptr_struct *dirptr,
 			       uint16 flags2,
diff --git a/source3/smbd/smb2_find.c b/source3/smbd/smb2_find.c
index 45b0890..af9995e 100644
--- a/source3/smbd/smb2_find.c
+++ b/source3/smbd/smb2_find.c
@@ -432,14 +432,13 @@ static struct tevent_req *smbd_smb2_find_send(TALLOC_CTX *mem_ctx,
 				     true);
 
 	while (true) {
-		bool ok;
 		bool got_exact_match = false;
 		bool out_of_space = false;
 		int space_remaining = in_output_buffer_length - off;
 
 		SMB_ASSERT(space_remaining >= 0);
 
-		ok = smbd_dirptr_lanman2_entry(state,
+		status = smbd_dirptr_lanman2_entry(state,
 					       conn,
 					       fsp->dptr,
 					       smbreq->flags2,
@@ -462,12 +461,18 @@ static struct tevent_req *smbd_smb2_find_send(TALLOC_CTX *mem_ctx,
 
 		off = (int)PTR_DIFF(pdata, base_data);
 
-		if (!ok) {
-			if (num > 0) {
+		if (!NT_STATUS_IS_OK(status)) {
+			if (NT_STATUS_EQUAL(status, NT_STATUS_ILLEGAL_CHARACTER)) {
+				/*
+				 * Bad character conversion on name. Ignore this
+				 * entry.
+				 */
+				continue;
+			} else if (num > 0) {
 				SIVAL(state->out_output_buffer.data, last_entry_off, 0);
 				tevent_req_done(req);
 				return tevent_req_post(req, ev);
-			} else if (out_of_space) {
+			} else if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
 				tevent_req_nterror(req, NT_STATUS_INFO_LENGTH_MISMATCH);
 				return tevent_req_post(req, ev);
 			} else {
diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index b950820..2d6c261 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -2212,7 +2212,7 @@ static NTSTATUS smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 	return NT_STATUS_OK;
 }
 
-bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
+NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 			       connection_struct *conn,
 			       struct dptr_struct *dirptr,
 			       uint16 flags2,
@@ -2279,7 +2279,7 @@ bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 				   &mode,
 				   &prev_dirpos);
 	if (!ok) {
-		return false;
+		return NT_STATUS_END_OF_FILE;
 	}
 
 	*got_exact_match = state.got_exact_match;
@@ -2306,14 +2306,14 @@ bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 	if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
 		*out_of_space = true;
 		dptr_SeekDir(dirptr, prev_dirpos);
-		return false;
+		return status;
 	}
 	if (!NT_STATUS_IS_OK(status)) {
-		return false;
+		return status;
 	}
 
 	*_last_entry_off = last_entry_off;
-	return true;
+	return NT_STATUS_OK;
 }
 
 static bool get_lanman2_dir_entry(TALLOC_CTX *ctx,
@@ -2337,13 +2337,14 @@ static bool get_lanman2_dir_entry(TALLOC_CTX *ctx,
 {
 	uint8_t align = 4;
 	const bool do_pad = true;
+	NTSTATUS status;
 
 	if (info_level >= 1 && info_level <= 3) {
 		/* No alignment on earlier info levels. */
 		align = 1;
 	}
 
-	return smbd_dirptr_lanman2_entry(ctx, conn, dirptr, flags2,
+	status = smbd_dirptr_lanman2_entry(ctx, conn, dirptr, flags2,
 					 path_mask, dirtype, info_level,
 					 requires_resume_key, dont_descend, ask_sharemode,
 					 align, do_pad,
@@ -2351,6 +2352,7 @@ static bool get_lanman2_dir_entry(TALLOC_CTX *ctx,
 					 space_remaining,
 					 out_of_space, got_exact_match,
 					 last_entry_off, name_list);
+	return NT_STATUS_IS_OK(status);
 }
 
 /****************************************************************************
-- 
2.1.0.rc2.206.gedb03e5


From 53f6fa8a624df8ad2fe7b7dcda966c26dd652b01 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 26 Aug 2014 14:54:56 -0700
Subject: [PATCH 08/11] s3: smbd: Remove unneeded 'out_of_space' parameter from
 smbd_dirptr_lanman2_entry().

This can now be handled by checking for the STATUS_MORE_ENTRIES error return.

Bug 10775 - smbd crashes when accessing garbage filenames

https://bugzilla.samba.org/show_bug.cgi?id=10775

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/globals.h   |  1 -
 source3/smbd/smb2_find.c |  2 --
 source3/smbd/trans2.c    | 10 ++++++----
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index 20ab75d..5a8e3bd 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -201,7 +201,6 @@ NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 			       char *base_data,
 			       char *end_data,
 			       int space_remaining,
-			       bool *out_of_space,
 			       bool *got_exact_match,
 			       int *_last_entry_off,
 			       struct ea_list *name_list);
diff --git a/source3/smbd/smb2_find.c b/source3/smbd/smb2_find.c
index af9995e..2dab86b 100644
--- a/source3/smbd/smb2_find.c
+++ b/source3/smbd/smb2_find.c
@@ -433,7 +433,6 @@ static struct tevent_req *smbd_smb2_find_send(TALLOC_CTX *mem_ctx,
 
 	while (true) {
 		bool got_exact_match = false;
-		bool out_of_space = false;
 		int space_remaining = in_output_buffer_length - off;
 
 		SMB_ASSERT(space_remaining >= 0);
@@ -454,7 +453,6 @@ static struct tevent_req *smbd_smb2_find_send(TALLOC_CTX *mem_ctx,
 					       base_data,
 					       end_data,
 					       space_remaining,
-					       &out_of_space,
 					       &got_exact_match,
 					       &last_entry_off,
 					       NULL);
diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 2d6c261..e4d64e8 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -2228,7 +2228,6 @@ NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 			       char *base_data,
 			       char *end_data,
 			       int space_remaining,
-			       bool *out_of_space,
 			       bool *got_exact_match,
 			       int *_last_entry_off,
 			       struct ea_list *name_list)
@@ -2251,7 +2250,6 @@ NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 	state.has_wild = dptr_has_wild(dirptr);
 	state.got_exact_match = false;
 
-	*out_of_space = false;
 	*got_exact_match = false;
 
 	p = strrchr_m(path_mask,'/');
@@ -2304,7 +2302,6 @@ NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 	TALLOC_FREE(fname);
 	TALLOC_FREE(smb_fname);
 	if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
-		*out_of_space = true;
 		dptr_SeekDir(dirptr, prev_dirpos);
 		return status;
 	}
@@ -2339,6 +2336,8 @@ static bool get_lanman2_dir_entry(TALLOC_CTX *ctx,
 	const bool do_pad = true;
 	NTSTATUS status;
 
+	*out_of_space = false;
+
 	if (info_level >= 1 && info_level <= 3) {
 		/* No alignment on earlier info levels. */
 		align = 1;
@@ -2350,8 +2349,11 @@ static bool get_lanman2_dir_entry(TALLOC_CTX *ctx,
 					 align, do_pad,
 					 ppdata, base_data, end_data,
 					 space_remaining,
-					 out_of_space, got_exact_match,
+					 got_exact_match,
 					 last_entry_off, name_list);
+	if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
+		*out_of_space = true;
+	}
 	return NT_STATUS_IS_OK(status);
 }
 
-- 
2.1.0.rc2.206.gedb03e5


From 9ee166738dd8e4b16143763a4ed20a7e47b84cd1 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 26 Aug 2014 15:05:24 -0700
Subject: [PATCH 09/11] s3: smbd: Change get_lanman2_dir_entry() to return the
 full NTSTATUS.

Handle the errors correctly at the level above inside the SMB1 server.

Bug 10775 - smbd crashes when accessing garbage filenames

https://bugzilla.samba.org/show_bug.cgi?id=10775

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/trans2.c | 52 +++++++++++++++++++++++++++++++--------------------
 1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index e4d64e8..6a66f3b 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -2313,7 +2313,7 @@ NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 	return NT_STATUS_OK;
 }
 
-static bool get_lanman2_dir_entry(TALLOC_CTX *ctx,
+static NTSTATUS get_lanman2_dir_entry(TALLOC_CTX *ctx,
 				connection_struct *conn,
 				struct dptr_struct *dirptr,
 				uint16 flags2,
@@ -2327,23 +2327,19 @@ static bool get_lanman2_dir_entry(TALLOC_CTX *ctx,
 				char *base_data,
 				char *end_data,
 				int space_remaining,
-				bool *out_of_space,
 				bool *got_exact_match,
 				int *last_entry_off,
 				struct ea_list *name_list)
 {
 	uint8_t align = 4;
 	const bool do_pad = true;
-	NTSTATUS status;
-
-	*out_of_space = false;
 
 	if (info_level >= 1 && info_level <= 3) {
 		/* No alignment on earlier info levels. */
 		align = 1;
 	}
 
-	status = smbd_dirptr_lanman2_entry(ctx, conn, dirptr, flags2,
+	return smbd_dirptr_lanman2_entry(ctx, conn, dirptr, flags2,
 					 path_mask, dirtype, info_level,
 					 requires_resume_key, dont_descend, ask_sharemode,
 					 align, do_pad,
@@ -2351,10 +2347,6 @@ static bool get_lanman2_dir_entry(TALLOC_CTX *ctx,
 					 space_remaining,
 					 got_exact_match,
 					 last_entry_off, name_list);
-	if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
-		*out_of_space = true;
-	}
-	return NT_STATUS_IS_OK(status);
 }
 
 /****************************************************************************
@@ -2628,7 +2620,7 @@ total_data=%u (should be %u)\n", (unsigned int)total_data, (unsigned int)IVAL(pd
 			out_of_space = True;
 			finished = False;
 		} else {
-			finished = !get_lanman2_dir_entry(ctx,
+			ntstatus = get_lanman2_dir_entry(ctx,
 					conn,
 					dirptr,
 					req->flags2,
@@ -2636,14 +2628,24 @@ total_data=%u (should be %u)\n", (unsigned int)total_data, (unsigned int)IVAL(pd
 					requires_resume_key,dont_descend,
 					ask_sharemode,
 					&p,pdata,data_end,
-					space_remaining, &out_of_space,
+					space_remaining,
 					&got_exact_match,
 					&last_entry_off, ea_list);
+			if (NT_STATUS_EQUAL(ntstatus,
+					NT_STATUS_ILLEGAL_CHARACTER)) {
+				/*
+				 * Bad character conversion on name. Ignore this
+				 * entry.
+				 */
+				continue;
+			}
+			if (NT_STATUS_EQUAL(ntstatus, STATUS_MORE_ENTRIES)) {
+				out_of_space = true;
+			} else {
+				finished = !NT_STATUS_IS_OK(ntstatus);
+			}
 		}
 
-		if (finished && out_of_space)
-			finished = False;
-
 		if (!finished && !out_of_space)
 			numentries++;
 
@@ -3004,7 +3006,7 @@ total_data=%u (should be %u)\n", (unsigned int)total_data, (unsigned int)IVAL(pd
 			out_of_space = True;
 			finished = False;
 		} else {
-			finished = !get_lanman2_dir_entry(ctx,
+			ntstatus = get_lanman2_dir_entry(ctx,
 						conn,
 						dirptr,
 						req->flags2,
@@ -3012,14 +3014,24 @@ total_data=%u (should be %u)\n", (unsigned int)total_data, (unsigned int)IVAL(pd
 						requires_resume_key,dont_descend,
 						ask_sharemode,
 						&p,pdata,data_end,
-						space_remaining, &out_of_space,
+						space_remaining,
 						&got_exact_match,
 						&last_entry_off, ea_list);
+			if (NT_STATUS_EQUAL(ntstatus,
+					NT_STATUS_ILLEGAL_CHARACTER)) {
+				/*
+				 * Bad character conversion on name. Ignore this
+				 * entry.
+				 */
+				continue;
+			}
+			if (NT_STATUS_EQUAL(ntstatus, STATUS_MORE_ENTRIES)) {
+				out_of_space = true;
+			} else {
+				finished = !NT_STATUS_IS_OK(ntstatus);
+			}
 		}
 
-		if (finished && out_of_space)
-			finished = False;
-
 		if (!finished && !out_of_space)
 			numentries++;
 
-- 
2.1.0.rc2.206.gedb03e5


From 353f7f6284efef4c8cf40f9297d1f65067ddd814 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 26 Aug 2014 16:39:56 -0700
Subject: [PATCH 10/11] s3: smbd: Fix a couple of tricky slow-path cases -
 don't return a mangled name for a name that cannot be converted.

For a name that contains an illegal Windows character, the
directory listing code returns the mangled 8.3 name as the
primary name for the file.

If the original (non-mangled) filename cannot be converted
to UCS2 on the wire via iconv due to conversion error, we
should skip that name when returning a directory listing,
as we can't map back from a returned 8.3 name to a usable
non-mangled filename if the client sends it back to us.

As this is only done in a very slow path (name must be mangled)
or in the old DOS protocol listing code I don't feel too bad
about using a talloc/free pair here.

Bug 10775 - smbd crashes when accessing garbage filenames

https://bugzilla.samba.org/show_bug.cgi?id=10775

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/dir.c    | 24 ++++++++++++++++++++++++
 source3/smbd/trans2.c | 25 +++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index 6c811fe..e60bc2c 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -1215,6 +1215,30 @@ static bool smbd_dirptr_8_3_match_fn(TALLOC_CTX *ctx,
 	    mangle_mask_match(conn, dname, mask)) {
 		char mname[13];
 		const char *fname;
+		/*
+		 * Ensure we can push the original name as UCS2. If
+		 * not, then just don't return this name.
+		 */
+		NTSTATUS status;
+		size_t ret_len = 0;
+		size_t len = (strlen(dname) + 2) * 4; /* Allow enough space. */
+		uint8_t *tmp = talloc_array(talloc_tos(),
+					uint8,
+					len);
+
+		status = srvstr_push(NULL,
+			FLAGS2_UNICODE_STRINGS,
+			tmp,
+			dname,
+			len,
+			STR_TERMINATE,
+			&ret_len);
+
+		TALLOC_FREE(tmp);
+
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 
 		if (!mangle_is_8_3(dname, false, conn->params)) {
 			bool ok = name_to_8_3(dname, mname, false,
diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 6a66f3b..44ff5fd 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -1469,6 +1469,31 @@ static bool smbd_dirptr_lanman2_match_fn(TALLOC_CTX *ctx,
 
 	/* Mangle fname if it's an illegal name. */
 	if (mangle_must_mangle(dname, state->conn->params)) {
+		/*
+		 * Slow path - ensure we can push the original name as UCS2. If
+		 * not, then just don't return this name.
+		 */
+		NTSTATUS status;
+		size_t ret_len = 0;
+		size_t len = (strlen(dname) + 2) * 4; /* Allow enough space. */
+		uint8_t *tmp = talloc_array(talloc_tos(),
+					uint8,
+					len);
+
+		status = srvstr_push(NULL,
+			FLAGS2_UNICODE_STRINGS,
+			tmp,
+			dname,
+			len,
+			STR_TERMINATE,
+			&ret_len);
+
+		TALLOC_FREE(tmp);
+
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
+
 		ok = name_to_8_3(dname, mangled_name,
 				 true, state->conn->params);
 		if (!ok) {
-- 
2.1.0.rc2.206.gedb03e5


From 8920efb2a2e862c6cebdd8afc3b5c4d0f59408b6 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 27 Aug 2014 13:15:29 -0700
Subject: [PATCH 11/11] Add test suite for iconv conversion fail of bad names
 over SMB1/SMB3.

Bug 10775 - smbd crashes when accessing garbage filenames

https://bugzilla.samba.org/show_bug.cgi?id=10775

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 selftest/target/Samba3.pm                 |  37 ++++++++
 source3/script/tests/test_smbclient_s3.sh | 144 ++++++++++++++++++++++++++++++
 2 files changed, 181 insertions(+)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 5544105..de40ced 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -873,6 +873,9 @@ sub provision($$$$$$)
 	my $msdfs_deeppath="$msdfs_shrdir/deeppath";
 	push(@dirs,$msdfs_deeppath);
 
+	my $badnames_shrdir="$shrdir/badnames";
+	push(@dirs,$badnames_shrdir);
+
 	# this gets autocreated by winbindd
 	my $wbsockdir="$prefix_abs/winbindd";
 	my $wbsockprivdir="$lockdir/winbindd_privileged";
@@ -925,6 +928,36 @@ sub provision($$$$$$)
 	symlink "msdfs:$server_ip\\ro-tmp", "$msdfs_shrdir/msdfs-src1";
 	symlink "msdfs:$server_ipv6\\ro-tmp", "$msdfs_shrdir/deeppath/msdfs-src2";
 
+	##
+	## create bad names in $badnames_shrdir
+	##
+	## (An invalid name, would be mangled to 8.3).
+        my $badname_target = "$badnames_shrdir/\340|\231\216\377\177";
+        unless (open(BADNAME_TARGET, ">$badname_target")) {
+                warn("Unable to open $badname_target");
+                return undef;
+        }
+        close(BADNAME_TARGET);
+        chmod 0666, $badname_target;
+
+	## (A bad name, would not be mangled to 8.3).
+        my $badname_target = "$badnames_shrdir/\240\276\346\327\377\177";
+        unless (open(BADNAME_TARGET, ">$badname_target")) {
+                warn("Unable to open $badname_target");
+                return undef;
+        }
+        close(BADNAME_TARGET);
+        chmod 0666, $badname_target;
+
+	## (A bad good name).
+        my $badname_target = "$badnames_shrdir/blank.txt";
+        unless (open(BADNAME_TARGET, ">$badname_target")) {
+                warn("Unable to open $badname_target");
+                return undef;
+        }
+        close(BADNAME_TARGET);
+        chmod 0666, $badname_target;
+
 	my $conffile="$libdir/server.conf";
 
 	my $nss_wrapper_pl = "$ENV{PERL} $self->{srcdir}/lib/nss_wrapper/nss_wrapper.pl";
@@ -1182,6 +1215,10 @@ sub provision($$$$$$)
 	fruit:metadata = netatalk
 	fruit:locking = netatalk
 	fruit:encoding = native
+
+[badname-tmp]
+	path = $badnames_shrdir
+	guest ok = yes
 	";
 	close(CONF);
 
diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh
index 596cd42..67ac94a 100755
--- a/source3/script/tests/test_smbclient_s3.sh
+++ b/source3/script/tests/test_smbclient_s3.sh
@@ -722,6 +722,146 @@ EOF
     fi
 }
 
+# Test accessing an share with bad names (won't convert).
+test_bad_names()
+{
+    cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/badname-tmp -I $SERVER_IP $ADDARGS -c ls 2>&1'
+    eval echo "$cmd"
+    out=`eval $cmd`
+    ret=$?
+
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed accessing badname-tmp (SMB1) with error $ret"
+	false
+	return
+    fi
+
+    echo "$out" | wc -l 2>&1 | grep 6
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - grep of number of lines (1) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep 'Domain=.*OS=.*Server='
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - grep (1) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep '^  \. *D'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - grep (2) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep '^  \.\. *D'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - grep (3) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep '^  blank.txt *N'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - grep (4) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep '^ *$'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - grep (5) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep 'blocks of size.*blocks available'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - grep (6) failed with $ret"
+	false
+    fi
+
+    # Now check again with -mSMB3
+    cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/badname-tmp -I $SERVER_IP -mSMB3 $ADDARGS -c ls 2>&1'
+    eval echo "$cmd"
+    out=`eval $cmd`
+    ret=$?
+
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed accessing badname-tmp (SMB3) with error $ret"
+	false
+	return
+    fi
+
+    echo "$out" | wc -l 2>&1 | grep 6
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - SMB3 grep of number of lines (1) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep 'Domain=.*OS=.*Server='
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - SMB3 grep (1) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep '^  \. *D'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - SMB3 grep (2) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep '^  \.\. *D'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - SMB3 grep (3) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep '^  blank.txt *N'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - SMB3 grep (4) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep '^ *$'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - SMB3 grep (5) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep 'blocks of size.*blocks available'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - SMB3 grep (6) failed with $ret"
+	false
+    fi
+}
 
 LOGDIR_PREFIX=test_smbclient_s3
 
@@ -798,6 +938,10 @@ testit "list with backup privilege" \
     test_backup_privilege_list || \
     failed=`expr $failed + 1`
 
+testit "list a share with bad names (won't convert)" \
+    test_bad_names || \
+    failed=`expr $failed + 1`
+
 testit "rm -rf $LOGDIR" \
     rm -rf $LOGDIR || \
     failed=`expr $failed + 1`
-- 
2.1.0.rc2.206.gedb03e5



More information about the samba-technical mailing list