All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] lib/LzmaEnc: Validate 'len' before subtracting
@ 2025-06-10 17:47 Lidong Chen via Grub-devel
  2025-06-10 17:58 ` sudhakar
  2025-06-10 18:13 ` Ross Philipson via Grub-devel
  0 siblings, 2 replies; 4+ messages in thread
From: Lidong Chen via Grub-devel @ 2025-06-10 17:47 UTC (permalink / raw)
  To: grub-devel, daniel.kiper; +Cc: Lidong Chen, ross.philipson

In LzmaEnc_CodeOneBlock(), both GetOptimumFast() and GetOptimum()
returns a value of greater or equal to 1, which is assigned to
'len'. But since LZMA_MATCH_LEN_MIN == 2, 'len' should be validated
before performing "len - LZMA_MATCH_LEN_MIN" to avoid underflow
when 'len' equals to 1.

Fixed: CID 51508

Signed-off-by: Lidong Chen <lidong.chen@oracle.com>
---
 grub-core/lib/LzmaEnc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/grub-core/lib/LzmaEnc.c b/grub-core/lib/LzmaEnc.c
index 52b331558..d74e96303 100644
--- a/grub-core/lib/LzmaEnc.c
+++ b/grub-core/lib/LzmaEnc.c
@@ -1880,6 +1880,11 @@ static SRes LzmaEnc_CodeOneBlock(CLzmaEnc *p, Bool useLimits, UInt32 maxPackSize
         UInt32 posSlot, lenToPosState;
         RangeEnc_EncodeBit(&p->rc, &p->isRep[p->state], 0);
         p->state = kMatchNextStates[p->state];
+	if (len < LZMA_MATCH_LEN_MIN)
+	  {
+	    p->result = SZ_ERROR_DATA;
+	    return CheckErrors(p);
+	  }
         LenEnc_Encode2(&p->lenEnc, &p->rc, len - LZMA_MATCH_LEN_MIN, posState, !p->fastMode, p->ProbPrices);
         pos -= LZMA_NUM_REPS;
         GetPosSlot(pos, posSlot);
-- 
2.34.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] lib/LzmaEnc: Validate 'len' before subtracting
  2025-06-10 17:47 [PATCH] lib/LzmaEnc: Validate 'len' before subtracting Lidong Chen via Grub-devel
@ 2025-06-10 17:58 ` sudhakar
  2025-06-10 18:13 ` Ross Philipson via Grub-devel
  1 sibling, 0 replies; 4+ messages in thread
From: sudhakar @ 2025-06-10 17:58 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: daniel.kiper, Lidong Chen, ross.philipson

On 2025-06-10 23:17, Lidong Chen via Grub-devel wrote:
> In LzmaEnc_CodeOneBlock(), both GetOptimumFast() and GetOptimum()
> returns a value of greater or equal to 1, which is assigned to
> 'len'. But since LZMA_MATCH_LEN_MIN == 2, 'len' should be validated
> before performing "len - LZMA_MATCH_LEN_MIN" to avoid underflow
> when 'len' equals to 1.
> 
> Fixed: CID 51508
> 
> Signed-off-by: Lidong Chen <lidong.chen@oracle.com>
> ---
>  grub-core/lib/LzmaEnc.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/grub-core/lib/LzmaEnc.c b/grub-core/lib/LzmaEnc.c
> index 52b331558..d74e96303 100644
> --- a/grub-core/lib/LzmaEnc.c
> +++ b/grub-core/lib/LzmaEnc.c
> @@ -1880,6 +1880,11 @@ static SRes LzmaEnc_CodeOneBlock(CLzmaEnc *p,
> Bool useLimits, UInt32 maxPackSize
>          UInt32 posSlot, lenToPosState;
>          RangeEnc_EncodeBit(&p->rc, &p->isRep[p->state], 0);
>          p->state = kMatchNextStates[p->state];
> +	if (len < LZMA_MATCH_LEN_MIN)
> +	  {
> +	    p->result = SZ_ERROR_DATA;
> +	    return CheckErrors(p);
> +	  }

Hi Lidong Chen,

please fix the indentation issue in if condition.

thanks,
sudhakar

>          LenEnc_Encode2(&p->lenEnc, &p->rc, len - LZMA_MATCH_LEN_MIN,
> posState, !p->fastMode, p->ProbPrices);
>          pos -= LZMA_NUM_REPS;
>          GetPosSlot(pos, posSlot);

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] lib/LzmaEnc: Validate 'len' before subtracting
  2025-06-10 17:47 [PATCH] lib/LzmaEnc: Validate 'len' before subtracting Lidong Chen via Grub-devel
  2025-06-10 17:58 ` sudhakar
@ 2025-06-10 18:13 ` Ross Philipson via Grub-devel
  2025-06-11  1:19   ` Lidong Chen via Grub-devel
  1 sibling, 1 reply; 4+ messages in thread
From: Ross Philipson via Grub-devel @ 2025-06-10 18:13 UTC (permalink / raw)
  To: Lidong Chen, grub-devel, daniel.kiper; +Cc: ross.philipson

On 6/10/25 10:47 AM, Lidong Chen wrote:
> In LzmaEnc_CodeOneBlock(), both GetOptimumFast() and GetOptimum()
> returns a value of greater or equal to 1, which is assigned to
> 'len'. But since LZMA_MATCH_LEN_MIN == 2, 'len' should be validated
> before performing "len - LZMA_MATCH_LEN_MIN" to avoid underflow
> when 'len' equals to 1.

It seems odd that these internal calls will produce values for len that 
the calling code can't use and it ends in an error. Does this happen 
when the input data is bad/malformed/etc? Is it considered an error 
condition down in those functions when they result in len being set to 1 
or 2?

Thanks
Ross

> 
> Fixed: CID 51508
> 
> Signed-off-by: Lidong Chen <lidong.chen@oracle.com>
> ---
>   grub-core/lib/LzmaEnc.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/grub-core/lib/LzmaEnc.c b/grub-core/lib/LzmaEnc.c
> index 52b331558..d74e96303 100644
> --- a/grub-core/lib/LzmaEnc.c
> +++ b/grub-core/lib/LzmaEnc.c
> @@ -1880,6 +1880,11 @@ static SRes LzmaEnc_CodeOneBlock(CLzmaEnc *p, Bool useLimits, UInt32 maxPackSize
>           UInt32 posSlot, lenToPosState;
>           RangeEnc_EncodeBit(&p->rc, &p->isRep[p->state], 0);
>           p->state = kMatchNextStates[p->state];
> +	if (len < LZMA_MATCH_LEN_MIN)
> +	  {
> +	    p->result = SZ_ERROR_DATA;
> +	    return CheckErrors(p);
> +	  }
>           LenEnc_Encode2(&p->lenEnc, &p->rc, len - LZMA_MATCH_LEN_MIN, posState, !p->fastMode, p->ProbPrices);
>           pos -= LZMA_NUM_REPS;
>           GetPosSlot(pos, posSlot);


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] lib/LzmaEnc: Validate 'len' before subtracting
  2025-06-10 18:13 ` Ross Philipson via Grub-devel
@ 2025-06-11  1:19   ` Lidong Chen via Grub-devel
  0 siblings, 0 replies; 4+ messages in thread
From: Lidong Chen via Grub-devel @ 2025-06-11  1:19 UTC (permalink / raw)
  To: Ross Philipson; +Cc: Lidong Chen, grub-devel@gnu.org, Daniel Kiper



> On Jun 10, 2025, at 11:13 AM, Ross Philipson <ross.philipson@oracle.com> wrote:
> 
> On 6/10/25 10:47 AM, Lidong Chen wrote:
>> In LzmaEnc_CodeOneBlock(), both GetOptimumFast() and GetOptimum()
>> returns a value of greater or equal to 1, which is assigned to
>> 'len'. But since LZMA_MATCH_LEN_MIN == 2, 'len' should be validated
>> before performing "len - LZMA_MATCH_LEN_MIN" to avoid underflow
>> when 'len' equals to 1.
> 
> It seems odd that these internal calls will produce values for len that the calling code can't use and it ends in an error. Does this happen when the input data is bad/malformed/etc? Is it considered an error condition down in those functions when they result in len being set to 1 or 2?

It looks to me that the call to GetOptimum() or GetOptimumFast() returns an encoding
decision, not the condition of the input data (line 1840 & 1871).  

    if (p->fastMode)
      len = GetOptimumFast(p, &pos);
    else
      len = GetOptimum(p, nowPos32, &pos);

    if (len == 1 && pos == 0xFFFFFFFF)
    {
1840  p->state = kLiteralNextStates[p->state];
    }
    else
    {
      if (pos < LZMA_NUM_REPS)
      {
           if (len == 1)
1871      p->state = kShortRepNextStates[p->state];
      }
      else
      {
        LenEnc_Encode2(&p->lenEnc, &p->rc, len - LZMA_MATCH_LEN_MIN, posState, !p->fastMode, p->ProbPrices);
      }
    }


Thanks,
Lidong


> 
> Thanks
> Ross
> 
>> Fixed: CID 51508
>> Signed-off-by: Lidong Chen <lidong.chen@oracle.com>
>> ---
>>  grub-core/lib/LzmaEnc.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>> diff --git a/grub-core/lib/LzmaEnc.c b/grub-core/lib/LzmaEnc.c
>> index 52b331558..d74e96303 100644
>> --- a/grub-core/lib/LzmaEnc.c
>> +++ b/grub-core/lib/LzmaEnc.c
>> @@ -1880,6 +1880,11 @@ static SRes LzmaEnc_CodeOneBlock(CLzmaEnc *p, Bool useLimits, UInt32 maxPackSize
>>          UInt32 posSlot, lenToPosState;
>>          RangeEnc_EncodeBit(&p->rc, &p->isRep[p->state], 0);
>>          p->state = kMatchNextStates[p->state];
>> + if (len < LZMA_MATCH_LEN_MIN)
>> +  {
>> +    p->result = SZ_ERROR_DATA;
>> +    return CheckErrors(p);
>> +  }
>>          LenEnc_Encode2(&p->lenEnc, &p->rc, len - LZMA_MATCH_LEN_MIN, posState, !p->fastMode, p->ProbPrices);
>>          pos -= LZMA_NUM_REPS;
>>          GetPosSlot(pos, posSlot);
> 

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-06-11  1:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-10 17:47 [PATCH] lib/LzmaEnc: Validate 'len' before subtracting Lidong Chen via Grub-devel
2025-06-10 17:58 ` sudhakar
2025-06-10 18:13 ` Ross Philipson via Grub-devel
2025-06-11  1:19   ` Lidong Chen via Grub-devel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.