Resolves: rhbz#2210193 CVE-2023-0950 Array Index UnderFlow in Calc Formula Parsing

This commit is contained in:
Stephan Bergmann 2023-06-20 13:31:17 +02:00
parent 6bdff2b679
commit a963104a00
4 changed files with 207 additions and 1 deletions

View File

@ -0,0 +1,77 @@
From d6bfde52b0b51e96075cfb195c2f9d8200a0fb08 Mon Sep 17 00:00:00 2001
From: Eike Rathke <erack@redhat.com>
Date: Thu, 16 Feb 2023 20:20:31 +0100
Subject: [PATCH 1/3] Obtain actual 0-parameter count for OR(), AND() and
1-parameter functions
OR and AND for legacy infix notation are classified as binary
operators but in fact are functions with parameter count. In case
no argument is supplied, GetByte() returns 0 and for that case the
implicit binary operator 2 parameters were wrongly assumed.
Similar for functions expecting 1 parameter, without argument 1
was assumed. For "real" unary and binary operators the compiler
already checks parameters. Omit OR and AND and 1-parameter
functions from this implicit assumption and return the actual 0
count.
Change-Id: Ie05398c112a98021ac2875cf7b6de994aee9d882
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147173
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Jenkins
(cherry picked from commit e7ce9bddadb2db222eaa5f594ef1de2e36d57e5c)
---
formula/source/core/api/token.cxx | 13 +++++--------
sc/source/core/tool/interpr4.cxx | 10 +++++++++-
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/formula/source/core/api/token.cxx b/formula/source/core/api/token.cxx
index 37dd26979ced..c2b12cf3a145 100644
--- a/formula/source/core/api/token.cxx
+++ b/formula/source/core/api/token.cxx
@@ -93,17 +93,14 @@ sal_uInt8 FormulaToken::GetParamCount() const
return 0; // parameters and specials
// ocIf... jump commands not for FAP, have cByte then
//2do: bool parameter whether FAP or not?
- else if ( GetByte() )
+ else if (GetByte())
return GetByte(); // all functions, also ocExternal and ocMacro
- else if (SC_OPCODE_START_BIN_OP <= eOp && eOp < SC_OPCODE_STOP_BIN_OP)
- return 2; // binary
- else if ((SC_OPCODE_START_UN_OP <= eOp && eOp < SC_OPCODE_STOP_UN_OP)
- || eOp == ocPercentSign)
- return 1; // unary
+ else if (SC_OPCODE_START_BIN_OP <= eOp && eOp < SC_OPCODE_STOP_BIN_OP && eOp != ocAnd && eOp != ocOr)
+ return 2; // binary operators, compiler checked; OR and AND legacy but are functions
+ else if ((SC_OPCODE_START_UN_OP <= eOp && eOp < SC_OPCODE_STOP_UN_OP) || eOp == ocPercentSign)
+ return 1; // unary operators, compiler checked
else if (SC_OPCODE_START_NO_PAR <= eOp && eOp < SC_OPCODE_STOP_NO_PAR)
return 0; // no parameter
- else if (SC_OPCODE_START_1_PAR <= eOp && eOp < SC_OPCODE_STOP_1_PAR)
- return 1; // one parameter
else if (FormulaCompiler::IsOpCodeJumpCommand( eOp ))
return 1; // only the condition counts as parameter
else
diff --git a/sc/source/core/tool/interpr4.cxx b/sc/source/core/tool/interpr4.cxx
index b9d34cd080a6..d5d8588fe49a 100644
--- a/sc/source/core/tool/interpr4.cxx
+++ b/sc/source/core/tool/interpr4.cxx
@@ -4022,7 +4022,15 @@ StackVar ScInterpreter::Interpret()
else if (sp >= pCur->GetParamCount())
nStackBase = sp - pCur->GetParamCount();
else
- nStackBase = sp; // underflow?!?
+ {
+ SAL_WARN("sc.core", "Stack anomaly at " << aPos.Format(
+ ScRefFlags::VALID | ScRefFlags::FORCE_DOC | ScRefFlags::TAB_3D, &mrDoc)
+ << " eOp: " << static_cast<int>(eOp)
+ << " params: " << static_cast<int>(pCur->GetParamCount())
+ << " nStackBase: " << nStackBase << " sp: " << sp);
+ nStackBase = sp;
+ assert(!"underflow");
+ }
}
switch( eOp )
--
2.41.0

View File

@ -0,0 +1,72 @@
From 94b5b99c96ad80e659ffa8dbe8045b65ab4cc791 Mon Sep 17 00:00:00 2001
From: Eike Rathke <erack@redhat.com>
Date: Fri, 17 Feb 2023 12:03:54 +0100
Subject: [PATCH 2/3] Stack check safety belt before fishing in muddy waters
Have it hit hard in debug builds.
Change-Id: I9ea54844a0661fd7a75616a2876983a74b2d5bad
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147205
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Jenkins
(cherry picked from commit 9d91fbba6f374fa1c10b38eae003da89bd4e6d4b)
---
sc/source/core/inc/interpre.hxx | 12 ++++++++++++
sc/source/core/tool/interpr1.cxx | 4 ++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/sc/source/core/inc/interpre.hxx b/sc/source/core/inc/interpre.hxx
index 4e986daf8453..3bcc9ef19fc2 100644
--- a/sc/source/core/inc/interpre.hxx
+++ b/sc/source/core/inc/interpre.hxx
@@ -235,6 +235,7 @@ private:
inline bool MustHaveParamCount( short nAct, short nMust );
inline bool MustHaveParamCount( short nAct, short nMust, short nMax );
inline bool MustHaveParamCountMin( short nAct, short nMin );
+ inline bool MustHaveParamCountMinWithStackCheck( short nAct, short nMin );
void PushParameterExpected();
void PushIllegalParameter();
void PushIllegalArgument();
@@ -1089,6 +1090,17 @@ inline bool ScInterpreter::MustHaveParamCountMin( short nAct, short nMin )
return false;
}
+inline bool ScInterpreter::MustHaveParamCountMinWithStackCheck( short nAct, short nMin )
+{
+ assert(sp >= nAct);
+ if (sp < nAct)
+ {
+ PushParameterExpected();
+ return false;
+ }
+ return MustHaveParamCountMin( nAct, nMin);
+}
+
inline bool ScInterpreter::CheckStringPositionArgument( double & fVal )
{
if (!std::isfinite( fVal))
diff --git a/sc/source/core/tool/interpr1.cxx b/sc/source/core/tool/interpr1.cxx
index 4f2789160a1c..5e2f36685024 100644
--- a/sc/source/core/tool/interpr1.cxx
+++ b/sc/source/core/tool/interpr1.cxx
@@ -7547,7 +7547,7 @@ void ScInterpreter::ScVLookup()
void ScInterpreter::ScSubTotal()
{
sal_uInt8 nParamCount = GetByte();
- if ( !MustHaveParamCountMin( nParamCount, 2 ) )
+ if ( !MustHaveParamCountMinWithStackCheck( nParamCount, 2 ) )
return;
// We must fish the 1st parameter deep from the stack! And push it on top.
@@ -7594,7 +7594,7 @@ void ScInterpreter::ScSubTotal()
void ScInterpreter::ScAggregate()
{
sal_uInt8 nParamCount = GetByte();
- if ( !MustHaveParamCountMin( nParamCount, 3 ) )
+ if ( !MustHaveParamCountMinWithStackCheck( nParamCount, 3 ) )
return;
const FormulaError nErr = nGlobalError;
--
2.41.0

View File

@ -0,0 +1,50 @@
From 0caab4f6da81346e54a2d4881ad52752071347ba Mon Sep 17 00:00:00 2001
From: Eike Rathke <erack@redhat.com>
Date: Mon, 27 Feb 2023 16:10:06 +0100
Subject: [PATCH 3/3] Always push a result, even if it's only an error
PERCENTILE() and QUARTILE() if an error was passed as argument (or
an error encountered during obtaining arguments) omitted to push
an error result, only setting the error.
Fallout from
commit f336f63da900d76c2bf6e5690f1c8a7bd15a0aa2
CommitDate: Thu Mar 3 16:28:59 2016 +0000
tdf#94635 Add FORECAST.ETS functions to Calc
Change-Id: I23e276fb0ce735cfd6383cc963446499dcf819f4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147922
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Jenkins
(cherry picked from commit 64914560e279c71ff1233f4bab851e2a292797e6)
---
sc/source/core/tool/interpr3.cxx | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sc/source/core/tool/interpr3.cxx b/sc/source/core/tool/interpr3.cxx
index 29c72f6f7280..cdbb4823a0e1 100644
--- a/sc/source/core/tool/interpr3.cxx
+++ b/sc/source/core/tool/interpr3.cxx
@@ -3481,7 +3481,7 @@ void ScInterpreter::ScPercentile( bool bInclusive )
GetNumberSequenceArray( 1, aArray, false );
if ( aArray.empty() || nGlobalError != FormulaError::NONE )
{
- SetError( FormulaError::NoValue );
+ PushNoValue();
return;
}
if ( bInclusive )
@@ -3504,7 +3504,7 @@ void ScInterpreter::ScQuartile( bool bInclusive )
GetNumberSequenceArray( 1, aArray, false );
if ( aArray.empty() || nGlobalError != FormulaError::NONE )
{
- SetError( FormulaError::NoValue );
+ PushNoValue();
return;
}
if ( bInclusive )
--
2.41.0

View File

@ -57,7 +57,7 @@ Summary: Free Software Productivity Suite
Name: libreoffice
Epoch: 1
Version: %{libo_version}.1
Release: 10%{?libo_prerelease}%{?dist}
Release: 11%{?libo_prerelease}%{?dist}
License: (MPLv1.1 or LGPLv3+) and LGPLv3 and LGPLv2+ and BSD and (MPLv1.1 or GPLv2 or LGPLv2 or Netscape) and Public Domain and ASL 2.0 and MPLv2.0 and CC0
URL: http://www.libreoffice.org/
@ -282,6 +282,9 @@ Patch26: 0005-CVE-2022-3140-Filter-out-unwanted-command-URIs.patch
Patch27: 0001-CVE-2022-38745.patch
Patch28: 0001-Don-t-use-Library_tl-in-URE-libraries.patch
Patch29: 0001-URE-Library_boostrap-should-not-depend-on-Library_co.patch
Patch30: 0001-Obtain-actual-0-parameter-count-for-OR-AND-and-1-par.patch
Patch31: 0002-Stack-check-safety-belt-before-fishing-in-muddy-wate.patch
Patch32: 0003-Always-push-a-result-even-if-it-s-only-an-error.patch
# not upstreamed
Patch500: 0001-disable-libe-book-support.patch
@ -2278,6 +2281,10 @@ gtk-update-icon-cache -q %{_datadir}/icons/hicolor &>/dev/null || :
%{_includedir}/LibreOfficeKit
%changelog
* Tue Jun 20 2023 Stephan Bergmann <sbergman@redhat.com> - 1:7.1.8.1-11 UNBUILT
- Resolves: rhbz#2210193 CVE-2023-0950 Array Index UnderFlow in Calc Formula
Parsing
* Tue May 16 2023 Stephan Bergmann <sbergman@redhat.com> - 1:7.1.8.1-10
- Fix erroneous libreoffice-ure dependencies