[PATCH] erofs-utils: lib: fix potential overflow issue

[Hongzhen Luo]

On 2024/8/6 02:39, Sandeep Dhavale wrote:
> On Sun, Aug 4, 2024 at 8:25 PM Hongzhen Luo<hongzhen at linux.alibaba.com>  wrote:
>> Coverity-id: 502377
>>
>> Signed-off-by: Hongzhen Luo<hongzhen at linux.alibaba.com>
>> ---
>>   lib/kite_deflate.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/kite_deflate.c b/lib/kite_deflate.c
>> index a5ebd66..e52e382 100644
>> --- a/lib/kite_deflate.c
>> +++ b/lib/kite_deflate.c
>> @@ -817,7 +817,8 @@ static const struct kite_matchfinder_cfg {
>>   /* 9 */ {32, 258, 258, 4096, true},    /* maximum compression */
>>   };
>>
>> -static int kite_mf_init(struct kite_matchfinder *mf, int wsiz, int level)
>> +static int kite_mf_init(struct kite_matchfinder *mf, unsigned int wsiz,
>> +                       int level)
>>   {
>>          const struct kite_matchfinder_cfg *cfg;
>>
>> --
>> 2.43.5
>>
> Hi Hongzhen,
> Can you please explain to me where the potential overflow is? Checkers
> can be smart so easy for me to miss.
> I see a below check in kitle_me_init()
>
>      if (wsiz > kHistorySize32 || (1 << ilog2(wsiz)) != wsiz)
>            return -EINVAL;
>
> So any larger value than kHistorySize32 which is (1U << 15) is already
> rejected. So what overflow case is this int => unsigned int type
> conversion solving?
>
> Thanks,
> Sandeep.

Hi Sandeep,

The coverity tool says that for code `mf->chain = 
malloc(sizeof(mf->chain[0]) * wsiz);` there is a potential overflow issue:

overflow_const: Expression 4UL * wsiz, which is equal to 
18446744065119617024, where wsiz is known to be equal to -2147483648, 
overflows the type that receives it, an unsigned integer 64 bits wide.

For example, when `wsiz` is set to -1, it is converted to an unsigned 
long value of 18446744073709551615, and multiplying this by 4 would lead 
to an overflow error. Consequently, I have defined wsiz as unsigned int, 
which has a maximum value of 4294967295. After converting this to 
unsigned long and multiplying by 4, an overflow will not occur.

In practical applications, however, `wsiz` would not take on such odd 
values.

---

Thanks,

Hongzhen Luo
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/linux-erofs/attachments/20240806/ae6749a2/attachment.htm>