* [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.