[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [bug-libunistring] libunistring-0.9.10: coverity scan shows a few mo
From: |
Mike FABIAN |
Subject: |
Re: [bug-libunistring] libunistring-0.9.10: coverity scan shows a few more problems in bundled gnulib |
Date: |
Thu, 02 May 2024 10:35:11 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
Bruno Haible <bruno@clisp.org> さんはかきました:
[...]
>> A new scan with Bruno’s patch applied now complains about several more
>> problems. I tried to have a look and as far as I understand it, most of
>> them could be false positives and no real problems. But I am not 100%
>> sure.
>
> libunistring version 0.9.10 is pretty old. Since then, several fixes have
> been applied to vasnprintf.c in particular:
>
> https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=278b4175c9d7dd47c1a3071554aac02add3b3c35
> https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=4d288a80bf7ebe29334b9805cdcc70eacb6059c1
> https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=5527d5c548702b89d217bbe58036996066a709b6
> https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=54c80fb6f106d7f3430dd075fcb7327bab07f368
> https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=ca1cd9b39787fe8a2329c77bc60d4a7c3ab2334e
> https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=c9e246f596836d1eb57120a2d3b65687170356a1
>
> It would therefore be better to do it with libunistring 1.2.
Yes.
> Also, for coverity findings on Gnulib source code, we use the coverity portal
> where we mark
> false positives etc. - so that when Gnulib source code is used in other
> packages, redundant
> work does not need to be done.
OK! I’ll try to use that in future!
[...]
[...]
>> Error: OVERRUN (CWE-119):
>> libunistring-0.9.10/lib/uninorm/canonical-decomposition.c:88: cond_at_most:
>> Checking "entry < 32768" implies that "entry" may be up to 32767 on the true
>> branch.
>> libunistring-0.9.10/lib/uninorm/canonical-decomposition.c:94: alias:
>> Assigning: "p" = "&gl_uninorm_decomp_chars_table[3 * entry]". "p"
>> may now point to as high as byte 98301 of
>> "gl_uninorm_decomp_chars_table" (which consists of 25575 bytes).
>> libunistring-0.9.10/lib/uninorm/canonical-decomposition.c:95: overrun-local:
>> Overrunning array of 25575 bytes at byte offset 98301 by dereferencing
>> pointer "p + 0".
>> # 93|
>> # 94| p = &gl_uninorm_decomp_chars_table[3 * entry];
>> # 95|-> element = (p[0] << 16) | (p[1] << 8) | p[2];
>> # 96| /* The first element has 5 bits for the decomposition
>> type. */
>> # 97| if (((element >> 18) & 0x1f) != UC_DECOMP_CANONICAL)
>>
>> gl_uninorm_decomp_chars_table is indeed only 25575 bytes long and if entry
>> can be up to 0x8000-1, then 3*entry could bee too big as an index for
>> gl_uninorm_decomp_chars_table
>
> We have extensive unit tests of this functionality, and when compiled with
> clang + ASAN, they don't trigger anything.
>
> In summary, while coverity can point to real bugs, nowadays clang + ASAN
> provides a better way (more findings with less effort) to check for mistakes.
So *all* of these errors are false positives. That is very
reassuring. Thank you *very* much for explaining this to me so well!
Yours,
Mike
--
Mike FABIAN <mfabian@redhat.com>
睡眠不足はいい仕事の敵だ。