Attempt to fix the charcnv issues causing nmbd to crash. If we get a failed

conversion simply copy as is. Also fixed the horrid malloc-twice-copy code
in the convert alloc path.
Jeremy.
(This used to be commit cfde7477fd12caef943a9422b52174438092a135)

3 files changed, 94 insertions, 36 deletions

diff --git a/source3/lib/charcnv.c b/source3/lib/charcnv.c
index ca5e378970..2b313d8ccf 100644
--- a/source3/lib/charcnv.c
+++ b/source3/lib/charcnv.c
@@ -154,6 +154,9 @@ void init_iconv(void)
  * @param dest pointer to destination string (multibyte or singlebyte)
  * @param destlen maximal length allowed for string
  * @returns the number of bytes occupied in the destination
+ *
+ * Ensure the srclen contains the terminating zero.
+ *
  **/
 size_t convert_string(charset_t from, charset_t to,
 		      void const *src, size_t srclen, 
@@ -167,16 +170,16 @@ size_t convert_string(charset_t from, charset_t to,
 
 	if (srclen == (size_t)-1)
 		srclen = strlen(src)+1;
+	if (srclen == 0)
+		return 0;
 
 	lazy_initialize_conv();
 
 	descriptor = conv_handles[from][to];
 
 	if (descriptor == (smb_iconv_t)-1 || descriptor == (smb_iconv_t)0) {
-		/* conversion not supported, use as is */
-		size_t len = MIN(srclen,destlen);
-		memcpy(dest,src,len);
-		return len;
+		DEBUG(0,("convert_string: Conversion not supported.\n"));
+		goto use_as_is;
 	}
 
 	i_len=srclen;
@@ -187,10 +190,11 @@ size_t convert_string(charset_t from, charset_t to,
 		switch(errno) {
 			case EINVAL:
 				reason="Incomplete multibyte sequence";
-				break;
+				DEBUG(3,("convert_string: Conversion error: %s(%s)\n",reason,inbuf));
+				goto use_as_is;
 			case E2BIG:
 				reason="No more room"; 
-				DEBUG(0, ("convert_string: Required %lu, available %lu\n",
+				DEBUG(3, ("convert_string: Required %lu, available %lu\n",
 					(unsigned long)srclen, (unsigned long)destlen));
 				/* we are not sure we need srclen bytes,
 			          may be more, may be less.
@@ -198,56 +202,86 @@ size_t convert_string(charset_t from, charset_t to,
 				  bytes ---simo */
 		               break;
 			case EILSEQ:
-			       reason="Illegal multibyte sequence";
-			       break;
+				reason="Illegal multibyte sequence";
+				DEBUG(3,("convert_string: Conversion error: %s(%s)\n",reason,inbuf));
+				goto use_as_is;
+			default:
+				DEBUG(0,("convert_string: Conversion error: %s(%s)\n",reason,inbuf));
+				break;
 		}
 		/* smb_panic(reason); */
 	}
 	return destlen-o_len;
+
+ use_as_is:
+
+	/* conversion not supported, use as is */
+	{
+		size_t len = MIN(srclen,destlen);
+		if (len)
+			memcpy(dest,src,len);
+		return len;
+	}
 }
 
 /**
  * Convert between character sets, allocating a new buffer for the result.
  *
+ * @param ctx TALLOC_CTX to use to allocate with. If NULL use malloc.
  * @param srclen length of source buffer.
  * @param dest always set at least to NULL
  * @note -1 is not accepted for srclen.
  *
  * @returns Size in bytes of the converted string; or -1 in case of error.
+ *
+ * Ensure the srclen contains the terminating zero.
  **/
 
-size_t convert_string_allocate(charset_t from, charset_t to,
+size_t convert_string_allocate(TALLOC_CTX *ctx, charset_t from, charset_t to,
 			       void const *src, size_t srclen, void **dest)
 {
-	size_t i_len, o_len, destlen;
+	size_t i_len, o_len, destlen = MAX(srclen, 512);
 	size_t retval;
 	const char *inbuf = (const char *)src;
-	char *outbuf, *ob;
+	char *outbuf = NULL, *ob = NULL;
 	smb_iconv_t descriptor;
 
 	*dest = NULL;
 
 	if (src == NULL || srclen == (size_t)-1)
 		return (size_t)-1;
+	if (srclen == 0)
+		return 0;
 
 	lazy_initialize_conv();
 
 	descriptor = conv_handles[from][to];
 
 	if (descriptor == (smb_iconv_t)-1 || descriptor == (smb_iconv_t)0) {
-		/* conversion not supported, return -1*/
-		DEBUG(3, ("convert_string_allocate: conversion not supported!\n"));
-		return -1;
+		DEBUG(0,("convert_string_allocate: Conversion not supported.\n"));
+		goto use_as_is;
 	}
 
-	destlen = MAX(srclen, 512);
-	outbuf = NULL;
 convert:
-	destlen = destlen * 2;
-	ob = (char *)Realloc(outbuf, destlen);
+	if ((destlen*2) < destlen) {
+		/* wrapped ! abort. */
+		DEBUG(0, ("convert_string_allocate: destlen wrapped !\n"));
+		if (!ctx)
+			SAFE_FREE(outbuf);
+		return (size_t)-1;
+	} else {
+		destlen = destlen * 2;
+	}
+
+	if (ctx)
+		ob = (char *)talloc_realloc(ctx, ob, destlen);
+	else
+		ob = (char *)Realloc(ob, destlen);
+
 	if (!ob) {
 		DEBUG(0, ("convert_string_allocate: realloc failed!\n"));
-		SAFE_FREE(outbuf);
+		if (!ctx)
+			SAFE_FREE(outbuf);
 		return (size_t)-1;
 	} else {
 		outbuf = ob;
@@ -262,12 +296,14 @@ convert:
 		switch(errno) {
 			case EINVAL:
 				reason="Incomplete multibyte sequence";
-				break;
+				DEBUG(3,("convert_string_allocate: Conversion error: %s(%s)\n",reason,inbuf));
+				goto use_as_is;
 			case E2BIG:
 				goto convert;		
 			case EILSEQ:
 				reason="Illegal multibyte sequence";
-				break;
+				DEBUG(3,("convert_string_allocate: Conversion error: %s(%s)\n",reason,inbuf));
+				goto use_as_is;
 		}
 		DEBUG(0,("Conversion error: %s(%s)\n",reason,inbuf));
 		/* smb_panic(reason); */
@@ -275,14 +311,41 @@ convert:
 	}
 	
 	destlen = destlen - o_len;
-	*dest = (char *)Realloc(ob,destlen);
+	if (ctx)
+		*dest = (char *)talloc_realloc(ctx,ob,destlen);
+	else
+		*dest = (char *)Realloc(ob,destlen);
 	if (destlen && !*dest) {
 		DEBUG(0, ("convert_string_allocate: out of memory!\n"));
-		SAFE_FREE(ob);
+		if (!ctx)
+			SAFE_FREE(ob);
 		return (size_t)-1;
 	}
 
 	return destlen;
+
+  use_as_is:
+
+	/* conversion not supported, use as is */
+	{
+		if (srclen && (destlen != srclen)) {
+			destlen = srclen;
+			if (ctx)
+				ob = (char *)talloc_realloc(ctx, ob, destlen);
+			else
+				ob = (char *)Realloc(ob, destlen);
+			if (!ob) {
+				DEBUG(0, ("convert_string_allocate: realloc failed!\n"));
+				if (!ctx)
+					SAFE_FREE(outbuf);
+				return (size_t)-1;
+			}
+		}
+		if (srclen && ob)
+			memcpy(ob,(const char *)src,srclen);
+		*dest = (char *)ob;
+		return srclen;
+	}
 }
 
 
@@ -298,17 +361,12 @@ convert:
 static size_t convert_string_talloc(TALLOC_CTX *ctx, charset_t from, charset_t to,
 		      		void const *src, size_t srclen, void **dest)
 {
-	void *alloced_string;
 	size_t dest_len;
 
-	/* FIXME: Ridiculous to allocate two buffers and then copy the string! */
-	
 	*dest = NULL;
-	dest_len=convert_string_allocate(from, to, src, srclen, &alloced_string);
+	dest_len=convert_string_allocate(ctx, from, to, src, srclen, dest);
 	if (dest_len == (size_t)-1)
 		return (size_t)-1;
-	*dest = talloc_memdup(ctx, alloced_string, dest_len);
-	SAFE_FREE(alloced_string);
 	if (*dest == NULL)
 		return (size_t)-1;
 	return dest_len;
@@ -365,7 +423,7 @@ size_t unix_strlower(const char *src, size_t srclen, char *dest, size_t destlen)
 	size_t size;
 	smb_ucs2_t *buffer;
 	
-	size = convert_string_allocate(CH_UNIX, CH_UCS2, src, srclen,
+	size = convert_string_allocate(NULL, CH_UNIX, CH_UCS2, src, srclen,
 				       (void **) &buffer);
 	if (size == -1) {
 		smb_panic("failed to create UCS2 buffer");
@@ -599,7 +657,7 @@ size_t push_ucs2_allocate(smb_ucs2_t **dest, const char *src)
 	size_t src_len = strlen(src)+1;
 
 	*dest = NULL;
-	return convert_string_allocate(CH_UNIX, CH_UCS2, src, src_len, (void **)dest);	
+	return convert_string_allocate(NULL, CH_UNIX, CH_UCS2, src, src_len, (void **)dest);	
 }
 
 /**
@@ -667,7 +725,7 @@ size_t push_utf8_allocate(char **dest, const char *src)
 	size_t src_len = strlen(src)+1;
 
 	*dest = NULL;
-	return convert_string_allocate(CH_UNIX, CH_UTF8, src, src_len, (void **)dest);	
+	return convert_string_allocate(NULL, CH_UNIX, CH_UTF8, src, src_len, (void **)dest);	
 }
 
 /**
@@ -755,7 +813,7 @@ size_t pull_ucs2_allocate(char **dest, const smb_ucs2_t *src)
 {
 	size_t src_len = (strlen_w(src)+1) * sizeof(smb_ucs2_t);
 	*dest = NULL;
-	return convert_string_allocate(CH_UCS2, CH_UNIX, src, src_len, (void **)dest);	
+	return convert_string_allocate(NULL, CH_UCS2, CH_UNIX, src, src_len, (void **)dest);	
 }
 
 /**
@@ -785,7 +843,7 @@ size_t pull_utf8_allocate(void **dest, const char *src)
 {
 	size_t src_len = strlen(src)+1;
 	*dest = NULL;
-	return convert_string_allocate(CH_UTF8, CH_UNIX, src, src_len, dest);	
+	return convert_string_allocate(NULL, CH_UTF8, CH_UNIX, src, src_len, dest);	
 }
  
 /**
diff --git a/source3/libsmb/climessage.c b/source3/libsmb/climessage.c
index 8ce8416487..035088212c 100644
--- a/source3/libsmb/climessage.c
+++ b/source3/libsmb/climessage.c
@@ -87,7 +87,7 @@ int cli_message_text_build(struct cli_state *cli, char *msg, int len, int grp)
 	p = smb_buf(cli->outbuf);
 	*p++ = 1;
 
-	if ((lendos = convert_string_allocate(CH_UNIX, CH_DOS, msg,len, (void **) &msgdos)) < 0 || !msgdos) {
+	if ((lendos = convert_string_allocate(NULL,CH_UNIX, CH_DOS, msg,len, (void **) &msgdos)) < 0 || !msgdos) {
 		DEBUG(3,("Conversion failed, sending message in UNIX charset\n"));
 		SSVAL(p, 0, len); p += 2;
 		memcpy(p, msg, len);
diff --git a/source3/smbd/message.c b/source3/smbd/message.c
index 233848d2d6..88f833e468 100644
--- a/source3/smbd/message.c
+++ b/source3/smbd/message.c
@@ -64,7 +64,7 @@ static void msg_deliver(void)
    * Incoming message is in DOS codepage format. Convert to UNIX.
    */
   
-  if ((len = convert_string_allocate(CH_DOS, CH_UNIX, msgbuf, msgpos, (void **) &msg)) < 0 || !msg) {
+  if ((len = convert_string_allocate(NULL,CH_DOS, CH_UNIX, msgbuf, msgpos, (void **) &msg)) < 0 || !msg) {
     DEBUG(3,("Conversion failed, delivering message in DOS codepage format\n"));
     for (i = 0; i < msgpos;) {
       if (msgbuf[i] == '\r' && i < (msgpos-1) && msgbuf[i+1] == '\n') {