util/charset/convert: do not overflow dest len in corner case
authorDouglas Bagnall <douglas.bagnall@catalyst.net.nz>
Fri, 10 May 2019 05:10:28 +0000 (17:10 +1200)
committerAndrew Bartlett <abartlet@samba.org>
Wed, 15 May 2019 04:03:37 +0000 (04:03 +0000)
Now, if destlen were SIZE_MAX - 1, destlen * 2 would wrap to SIZE_MAX - 3,
which makes (destlen * 2 + 2) == SIZE_MAX - 1, the same number again.
So we need the <= comparison in this case.

As things stand, it is not actually possible for destlen to be
SIZE_MAX (because it is always an even number after the first round,
and the first round is constrained to be < SIZE_MAX / 2, but *if*
destlen was SIZE_MAX, destlen * 2 + 2 would be 0, so that case is OK.
Similarly the SIZE_MAX - 2 and smaller cases were covered by the
original formula.

We add the comment for people who are wondering WTF is going on with
all this destlen manipulation.

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

index afc8f5c4db2338ea86afebeb195416e9e2fcb8a2..459121426788d3d2a8deb1731d4a8945b3c52668 100644 (file)
@@ -406,10 +406,10 @@ bool convert_string_talloc_handle(TALLOC_CTX *ctx, struct smb_iconv_handle *ic,
        }
        destlen = srclen * 3 / 2;
 
-  convert:
+  convert: /* this is a do-while loop with case E2BIG below. */
 
        /* +2 is for ucs2 null termination. */
-       if ((destlen*2)+2 < destlen) {
+       if ((destlen*2)+2 <= destlen) {
                /* wrapped ! abort. */
                DEBUG(0, ("convert_string_talloc: destlen wrapped !\n"));
                TALLOC_FREE(outbuf);