Re: Fwd: probable bug(s) in do_conversion() in iri.c

[Derek Martin]

I believe the following patch correctly fixes the reallocation case
and avoids any buffer overflow. The variable tooshort is removed
because it was only set, never used.  After looking at the EILSEQ ||
EINVAL case again, I think it does not need to be modified in order to
maintain what it was doing, though TBH I still don't quite get the
point of what it's doing there.


diff -ur wget-1.21.1.orig/src/iri.c wget-1.21.1/src/iri.c
--- wget-1.21.1.orig/src/iri.c  2021-01-08 17:51:43.000000000 -0500
+++ wget-1.21.1/src/iri.c   2021-04-20 17:06:58.356896518 -0400
@@ -130,9 +130,9 @@
 {
   iconv_t cd;
   /* sXXXav : hummm hard to guess... */
-  size_t len, done, outlen;
+  size_t outbuf_len = 0, out_remain = 0, converted = 0;
   int invalid = 0, tooshort = 0;
-  char *s, *in, *in_save;
+  char *outcur, *in, *in_save;

   cd = iconv_open (tocode, fromcode);
   if (cd == (iconv_t)(-1))
@@ -148,17 +148,16 @@
   url_unescape_except_reserved (in);
   inlen = strlen(in);

-  len = outlen = inlen * 2;
-  *out = s = xmalloc (outlen + 1);
-  done = 0;
+  /* Leave 4 bytes for null for e.g. UTF-32 */
+  outbuf_len = out_remain = inlen * 2;
+  *out = outcur = xmalloc (outlen + 4);

   for (;;)
     {
-      if (iconv (cd, (ICONV_CONST char **) &in, &inlen, out, &outlen) != 
(size_t)(-1) &&
+      if (iconv (cd, (ICONV_CONST char **) &in, &inlen, &outcur, &outlen) != 
(size_t)(-1) &&
           iconv (cd, NULL, NULL, out, &outlen) != (size_t)(-1))
         {
-          *out = s;
-          *(s + len - outlen - done) = '\0';
+          for (int i = 0; i < 4; ++i) *(outcur + i) = '\0';
           xfree(in_save);
           iconv_close(cd);
           IF_DEBUG
@@ -188,12 +187,12 @@
         }
       else if (errno == E2BIG) /* Output buffer full */
         {
-          tooshort++;
-          done = len;
-          len = done + inlen * 2;
-          s = xrealloc (s, len + 1);
-          *out = s + done - outlen;
-          outlen += inlen * 2;
+          converted = outbuf_len - out_remain;
+          outbuf_len = converted + in_remain * 2;
+          /* again, leave 4 bytes for null */
+          *out = xrealloc (*out, outbuf_len + 4);
+          outcur = *out + converted;
+          out_remain = outbuf_len - converted;
         }
       else /* Weird, we got an unspecified error */
         {