util/charset/convert: do not pretend to realloc
authorDouglas Bagnall <douglas.bagnall@catalyst.net.nz>
Fri, 10 May 2019 07:37:54 +0000 (19:37 +1200)
committerAndrew Bartlett <abartlet@samba.org>
Wed, 15 May 2019 04:03:37 +0000 (04:03 +0000)
It seems very likely that our clever attempts to dynamically realloc
the output buffer were never triggered. Two lines of reasoning lead to
this conclusion:

1. We allocate 3 * srclen to start with, but no conversion we use will
   more than that. To be precise, from 8-bit charsets we will only deal
   with codepoints in the Unicode basic multilingual plane (up to 0xFFFF).

   These can all be expressed as 3 or fewer utf-8 bytes. In UTF16 they
   are naturally 2 bytes, while in the DOS codes they are 1 byte.

   We have checked the code tables, and can not find a plausible
   (e.g. not EBCDIC) DOS code page or unix charset that is outside
   this range.  Clients cannot chose the code page, the only code
   pages we will use come from 'unix charset' and 'dos charset'
   smb.conf parameters.

   Therefore the worst that can possibly happen is we expand 1 byte into 3
   (specifically, when converting some e.g. CP850 codepoints to UTF-8).

2. If the reallocation was ever used, the results would have been
   catastrophically wrong, as the input pointer was not reset.

Therefore we skip the complication of the goto loop and let E2BIG be
just another impossible error to report.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
lib/util/charset/convert_string.c

index baff3d9e30882b2b184d1beab7014b549108bf7e..d8fb9bdd477bbff2a8cb475d992a5e8333832bc1 100644 (file)
@@ -397,27 +397,14 @@ bool convert_string_talloc_handle(TALLOC_CTX *ctx, struct smb_iconv_handle *ic,
                return false;
        }
 
-       if (srclen >= SIZE_MAX / 3) {
+       if (srclen >= (SIZE_MAX - 2) / 3) {
                DBG_ERR("convert_string_talloc: "
                        "srclen is %zu, destlen would wrap!\n",
                        srclen);
                errno = EOPNOTSUPP;
                return false;
        }
-       destlen = srclen * 3 / 2;
-
-  convert: /* this is a do-while loop with case E2BIG below. */
-
-       /* +2 is for ucs2 null termination. */
-       if ((destlen*2)+2 <= destlen) {
-               /* wrapped ! abort. */
-               DEBUG(0, ("convert_string_talloc: destlen wrapped !\n"));
-               TALLOC_FREE(outbuf);
-               errno = EOPNOTSUPP;
-               return false;
-       } else {
-               destlen = destlen * 2;
-       }
+       destlen = srclen * 3;
 
        /* +2 is for ucs2 null termination. */
        ob = talloc_realloc(ctx, ob, char, destlen + 2);
@@ -443,7 +430,11 @@ bool convert_string_talloc_handle(TALLOC_CTX *ctx, struct smb_iconv_handle *ic,
                                DEBUG(3,("convert_string_talloc: Conversion error: %s(%s)\n",reason,inbuf));
                                break;
                        case E2BIG:
-                               goto convert;
+                               reason = "output buffer is too small";
+                               DBG_NOTICE("convert_string_talloc: "
+                                          "Conversion error: %s(%s)\n",
+                                          reason, inbuf);
+                               break;
                        case EILSEQ:
                                reason="Illegal multibyte sequence";
                                DEBUG(3,("convert_string_talloc: Conversion error: %s(%s)\n",reason,inbuf));