Mike FABIAN wrote in
<https://lists.gnu.org/archive/html/bug-libunistring/2021-06/msg00000.html>
> A coverity scan of libunistring showed the following 4 problems in the
> bundled gnulib:
>
>
> Error: RESOURCE_LEAK (CWE-772):
> libunistring-0.9.10/lib/vasnprintf.c:2123: alloc_fn: Storage is returned
> from allocation function "u8_to_u32".
> libunistring-0.9.10/lib/vasnprintf.c:2123: var_assign: Assigning:
> "converted" = storage returned from "u8_to_u32(arg, arg_end - arg, converted,
> &converted_len)".
> libunistring-0.9.10/lib/vasnprintf.c:2140: leaked_storage: Variable
> "converted" going out of scope leaks the storage it points to.
> # 2138| if (converted != result + length)
> # 2139| {
> # 2140|-> ENSURE_ALLOCATION (xsum (length,
> converted_len));
> # 2141| DCHAR_CPY (result + length,
> converted, converted_len);
> # 2142| free (converted);
>
> Here in the ENSURE_ALLOCATION macro, if malloc or realloc fails, the macro
> does a “goto out_of_memory;” and then “converted” goes out of scope and is
> not freed anymore.
>
> The other 3 reported problems are the same:
>
> Error: RESOURCE_LEAK (CWE-772):
> libunistring-0.9.10/lib/vasnprintf.c:2249: alloc_fn: Storage is returned
> from allocation function "u16_to_u32".
> libunistring-0.9.10/lib/vasnprintf.c:2249: var_assign: Assigning:
> "converted" = storage returned from "u16_to_u32(arg, arg_end - arg,
> converted, &converted_len)".
> libunistring-0.9.10/lib/vasnprintf.c:2266: leaked_storage: Variable
> "converted" going out of scope leaks the storage it points to.
> # 2264| if (converted != result + length)
> # 2265| {
> # 2266|-> ENSURE_ALLOCATION (xsum (length,
> converted_len));
> # 2267| DCHAR_CPY (result + length,
> converted, converted_len);
> # 2268| free (converted);
>
> Error: RESOURCE_LEAK (CWE-772):
> libunistring-0.9.10/lib/vasnprintf.c:2375: alloc_fn: Storage is returned
> from allocation function "u32_to_u16".
> libunistring-0.9.10/lib/vasnprintf.c:2375: var_assign: Assigning:
> "converted" = storage returned from "u32_to_u16(arg, arg_end - arg,
> converted, &converted_len)".
> libunistring-0.9.10/lib/vasnprintf.c:2392: leaked_storage: Variable
> "converted" going out of scope leaks the storage it points to.
> # 2390| if (converted != result + length)
> # 2391| {
> # 2392|-> ENSURE_ALLOCATION (xsum (length,
> converted_len));
> # 2393| DCHAR_CPY (result + length,
> converted, converted_len);
> # 2394| free (converted);
>
> Error: RESOURCE_LEAK (CWE-772):
> libunistring-0.9.10/lib/vasnprintf.c:5354: alloc_fn: Storage is returned
> from allocation function "u8_conv_from_encoding".
> libunistring-0.9.10/lib/vasnprintf.c:5354: var_assign: Assigning:
> "tmpdst" = storage returned from "u8_conv_from_encoding(locale_charset(),
> iconveh_question_mark, tmpsrc, count, NULL, NULL, &tmpdst_len)".
> libunistring-0.9.10/lib/vasnprintf.c:5371: leaked_storage: Variable
> "tmpdst" going out of scope leaks the storage it points to.
> # 5369| return NULL;
> # 5370| }
> # 5371|-> ENSURE_ALLOCATION (xsum (length,
> tmpdst_len));
> # 5372| DCHAR_CPY (result + length, tmpdst,
> tmpdst_len);
> # 5373| free (tmpdst);
Thanks for the report. Fixed through this patch (in gnulib).
2021-06-05 Bruno Haible <br...@clisp.org>
vasnprintf: Don't leak memory when memory allocation fails.
Found by Coverity. Reported by Mike Fabian <mfab...@redhat.com> in
<https://lists.gnu.org/archive/html/bug-libunistring/2021-06/msg00000.html>.
* lib/vasnprintf.c (VASNPRINTF): In places where a local variable points
to heap-allocated storage, free that storage before doing
'goto out_of_memory;'.
diff --git a/lib/vasnprintf.c b/lib/vasnprintf.c
index f859fc4..089c113 100644
--- a/lib/vasnprintf.c
+++ b/lib/vasnprintf.c
@@ -1924,7 +1924,7 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
/* Ensures that allocated >= needed. Aborts through a jump to
out_of_memory if needed is SIZE_MAX or otherwise too big. */
-#define ENSURE_ALLOCATION(needed) \
+#define ENSURE_ALLOCATION_ELSE(needed, oom_statement) \
if ((needed) > allocated) \
{ \
size_t memory_size; \
@@ -1935,17 +1935,19 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
allocated = (needed); \
memory_size = xtimes (allocated, sizeof (DCHAR_T)); \
if (size_overflow_p (memory_size)) \
- goto out_of_memory; \
+ oom_statement \
if (result == resultbuf || result == NULL) \
memory = (DCHAR_T *) malloc (memory_size); \
else \
memory = (DCHAR_T *) realloc (result, memory_size); \
if (memory == NULL) \
- goto out_of_memory; \
+ oom_statement \
if (result == resultbuf && length > 0) \
DCHAR_CPY (memory, result, length); \
result = memory; \
}
+#define ENSURE_ALLOCATION(needed) \
+ ENSURE_ALLOCATION_ELSE((needed), goto out_of_memory; )
for (cp = format, i = 0, dp = &d.dir[0]; ; cp = dp->dir_end, i++, dp++)
{
@@ -2193,7 +2195,8 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
}
if (converted != result + length)
{
- ENSURE_ALLOCATION (xsum (length, converted_len));
+ ENSURE_ALLOCATION_ELSE (xsum (length,
converted_len),
+ { free (converted); goto
out_of_memory; });
DCHAR_CPY (result + length, converted,
converted_len);
free (converted);
}
@@ -2317,7 +2320,8 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
}
if (converted != result + length)
{
- ENSURE_ALLOCATION (xsum (length, converted_len));
+ ENSURE_ALLOCATION_ELSE (xsum (length,
converted_len),
+ { free (converted); goto
out_of_memory; });
DCHAR_CPY (result + length, converted,
converted_len);
free (converted);
}
@@ -2441,7 +2445,8 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
}
if (converted != result + length)
{
- ENSURE_ALLOCATION (xsum (length, converted_len));
+ ENSURE_ALLOCATION_ELSE (xsum (length,
converted_len),
+ { free (converted); goto
out_of_memory; });
DCHAR_CPY (result + length, converted,
converted_len);
free (converted);
}
@@ -2944,7 +2949,8 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
}
}
# else
- ENSURE_ALLOCATION (xsum (length, tmpdst_len));
+ ENSURE_ALLOCATION_ELSE (xsum (length, tmpdst_len),
+ { free (tmpdst); goto out_of_memory;
});
DCHAR_CPY (result + length, tmpdst, tmpdst_len);
free (tmpdst);
length += tmpdst_len;
@@ -3147,7 +3153,8 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
}
}
# else
- ENSURE_ALLOCATION (xsum (length, tmpdst_len));
+ ENSURE_ALLOCATION_ELSE (xsum (length, tmpdst_len),
+ { free (tmpdst); goto out_of_memory;
});
DCHAR_CPY (result + length, tmpdst, tmpdst_len);
free (tmpdst);
length += tmpdst_len;
@@ -5598,7 +5605,8 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
CLEANUP ();
return NULL;
}
- ENSURE_ALLOCATION (xsum (length, tmpdst_len));
+ ENSURE_ALLOCATION_ELSE (xsum (length, tmpdst_len),
+ { free (tmpdst); goto
out_of_memory; });
DCHAR_CPY (result + length, tmpdst, tmpdst_len);
free (tmpdst);
count = tmpdst_len;
