Resolves: rhbz#2210191 CVE-2023-0950 Array Index UnderFlow in Calc Formula Parsing
This commit is contained in:
		
							parent
							
								
									97992112e9
								
							
						
					
					
						commit
						5faf9f0999
					
				| @ -0,0 +1,80 @@ | ||||
| From b66d735cf3dc8b80783cb161c0aff5b990db1bb0 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) | ||||
| Conflicts: | ||||
|         sc/source/core/tool/interpr4.cxx | ||||
| ---
 | ||||
|  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 17594207234f..0db0c3464610 100644
 | ||||
| --- a/formula/source/core/api/token.cxx
 | ||||
| +++ b/formula/source/core/api/token.cxx
 | ||||
| @@ -95,17 +95,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 eb3fb987c034..94235c33eaef 100644
 | ||||
| --- a/sc/source/core/tool/interpr4.cxx
 | ||||
| +++ b/sc/source/core/tool/interpr4.cxx
 | ||||
| @@ -4012,7 +4012,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, pDok)
 | ||||
| +                            << "  eOp: " << static_cast<int>(eOp)
 | ||||
| +                            << "  params: " << static_cast<int>(pCur->GetParamCount())
 | ||||
| +                            << "  nStackBase: " << nStackBase << "  sp: " << sp);
 | ||||
| +                    nStackBase = sp;
 | ||||
| +                    assert(!"underflow");
 | ||||
| +                }
 | ||||
|              } | ||||
|   | ||||
|              switch( eOp ) | ||||
| -- 
 | ||||
| 2.41.0 | ||||
| 
 | ||||
| @ -0,0 +1,82 @@ | ||||
| From c0e926365dc7651dcb5eee48f50e6990523662ad 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 | ||||
| MIME-Version: 1.0 | ||||
| Content-Type: text/plain; charset=UTF-8 | ||||
| Content-Transfer-Encoding: 8bit | ||||
| 
 | ||||
| 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) | ||||
| Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147245 | ||||
| Reviewed-by: Caolán McNamara <caolanm@redhat.com> | ||||
| (cherry picked from commit 166a07062dd4ffedca6106f439a6fcddaeee5eb5) | ||||
| Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147391 | ||||
| Tested-by: Michael Stahl <michael.stahl@allotropia.de> | ||||
| Reviewed-by: Michael Stahl <michael.stahl@allotropia.de> | ||||
| (cherry picked from commit f8efb098f2abbf054a15dcf7daaaacfa575685ae) | ||||
| ---
 | ||||
|  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 3b902524d901..c7d4527dbf57 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(); | ||||
| @@ -1086,6 +1087,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 (!rtl::math::isFinite( fVal)) | ||||
| diff --git a/sc/source/core/tool/interpr1.cxx b/sc/source/core/tool/interpr1.cxx
 | ||||
| index e375f1626ec5..4b093cb62d4f 100644
 | ||||
| --- a/sc/source/core/tool/interpr1.cxx
 | ||||
| +++ b/sc/source/core/tool/interpr1.cxx
 | ||||
| @@ -7524,7 +7524,7 @@ void ScInterpreter::ScVLookup()
 | ||||
|  void ScInterpreter::ScSubTotal() | ||||
|  { | ||||
|      sal_uInt8 nParamCount = GetByte(); | ||||
| -    if ( MustHaveParamCountMin( nParamCount, 2 ) )
 | ||||
| +    if ( MustHaveParamCountMinWithStackCheck( nParamCount, 2 ) )
 | ||||
|      { | ||||
|          // We must fish the 1st parameter deep from the stack! And push it on top. | ||||
|          const FormulaToken* p = pStack[ sp - nParamCount ]; | ||||
| @@ -7571,7 +7571,7 @@ void ScInterpreter::ScSubTotal()
 | ||||
|  void ScInterpreter::ScAggregate() | ||||
|  { | ||||
|      sal_uInt8 nParamCount = GetByte(); | ||||
| -    if ( MustHaveParamCountMin( nParamCount, 3 ) )
 | ||||
| +    if ( MustHaveParamCountMinWithStackCheck( nParamCount, 3 ) )
 | ||||
|      { | ||||
|          // fish the 1st parameter from the stack and push it on top. | ||||
|          const FormulaToken* p = pStack[ sp - nParamCount ]; | ||||
| -- 
 | ||||
| 2.41.0 | ||||
| 
 | ||||
							
								
								
									
										50
									
								
								0003-Always-push-a-result-even-if-it-s-only-an-error.patch
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										50
									
								
								0003-Always-push-a-result-even-if-it-s-only-an-error.patch
									
									
									
									
									
										Normal file
									
								
							| @ -0,0 +1,50 @@ | ||||
| From 7e128f02a7cb513e4e57dbb1970fa316f456aa45 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 f219beca9386..d442d4eee224 100644
 | ||||
| --- a/sc/source/core/tool/interpr3.cxx
 | ||||
| +++ b/sc/source/core/tool/interpr3.cxx
 | ||||
| @@ -3474,7 +3474,7 @@ void ScInterpreter::ScPercentile( bool bInclusive )
 | ||||
|      GetNumberSequenceArray( 1, aArray, false ); | ||||
|      if ( aArray.empty() || nGlobalError != FormulaError::NONE ) | ||||
|      { | ||||
| -        SetError( FormulaError::NoValue );
 | ||||
| +        PushNoValue();
 | ||||
|          return; | ||||
|      } | ||||
|      if ( bInclusive ) | ||||
| @@ -3497,7 +3497,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 | ||||
| 
 | ||||
| @ -54,7 +54,7 @@ Summary:        Free Software Productivity Suite | ||||
| Name:           libreoffice | ||||
| Epoch:          1 | ||||
| Version:        %{libo_version}.2 | ||||
| Release:        14%{?libo_prerelease}%{?dist} | ||||
| Release:        15%{?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/ | ||||
| 
 | ||||
| @ -286,6 +286,9 @@ Patch42: 0003-CVE-2022-26306-add-Initialization-Vectors-to-passwor.patch | ||||
| Patch43: 0004-CVE-2022-2630-6-7-add-infobar-to-prompt-to-refresh-t.patch | ||||
| Patch44: 0001-CVE-2022-3140.patch | ||||
| Patch45: 0001-CVE-2022-38745.patch | ||||
| Patch46: 0001-Obtain-actual-0-parameter-count-for-OR-AND-and-1-par.patch | ||||
| Patch47: 0002-Stack-check-safety-belt-before-fishing-in-muddy-wate.patch | ||||
| Patch48: 0003-Always-push-a-result-even-if-it-s-only-an-error.patch | ||||
| 
 | ||||
| %if 0%{?rhel} | ||||
| # not upstreamed | ||||
| @ -2288,6 +2291,10 @@ done | ||||
| %{_includedir}/LibreOfficeKit | ||||
| 
 | ||||
| %changelog | ||||
| * Mon Jun 19 2023 Stephan Bergmann <sbergman@redhat.com> - 1:6.4.7.2-15 UNBUILT | ||||
| - Resolves: rhbz#2210191 CVE-2023-0950 Array Index UnderFlow in Calc Formula | ||||
|   Parsing | ||||
| 
 | ||||
| * Wed Apr 12 2023 Caolán McNamara <caolanm@redhat.com> - 1:6.4.7.2-14 | ||||
| - Resolves: rhbz#2182390 CVE-2022-38745 Empty entry in Java class path | ||||
| 
 | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user