bug-libunistring
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug-libunistring] coverity scan shows 4 problems in bundled gnulib


From: Mike FABIAN
Subject: [bug-libunistring] coverity scan shows 4 problems in bundled gnulib
Date: Wed, 02 Jun 2021 09:38:25 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux)

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);

This looks a bit awkward to fix, I wonder how it could be done.

Maybe one could add a second parameter to the macro ENSURE_ALLOCATION and pass
a pointer which should be freed in case the allocation fails?

Should I try to make a patch which does this?

-- 
Mike FABIAN <mfabian@redhat.com>
睡眠不足はいい仕事の敵だ。




reply via email to

[Prev in Thread] Current Thread [Next in Thread]