Subject: CVE-2023-0950 Obtain actual 0-parameter count for OR(), AND() and 1...

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)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147129
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
(cherry picked from commit d6599a2af131994487d2d9223a4fd32a8c3ddc49)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147390
Tested-by: Michael Stahl <michael.stahl@allotropia.de>
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
bug: https://www.libreoffice.org/about-us/security/advisories/cve-2023-0950/
bug-debian-security: https://deb.freexian.com/extended-lts/tracker/CVE-2023-0950

Gbp-Pq: Name 0076-Subject-CVE-2023-0950-Obtain-actual-0-parameter-coun.patch

diff --git a/formula/source/core/api/token.cxx b/formula/source/core/api/token.cxx
index a8fb0b42541de36dcca94456d309c994ee283959..5e6068550e3f421b1cda2ec76ad344e21b65f49a 100644 (file)
--- a/formula/source/core/api/token.cxx
+++ b/formula/source/core/api/token.cxx
@@ -105,17 +105,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 32238916e162c1631fc8303acd009c118f4381f4..d234ed16d896b0a37d343fb8fd9e64628920e56f 100644 (file)
--- a/sc/source/core/tool/interpr4.cxx
+++ b/sc/source/core/tool/interpr4.cxx
@@ -4029,11 +4029,19 @@ StackVar ScInterpreter::Interpret()
                     eOp = ocNone;       // JumpMatrix created
                     nStackBase = sp;
                 }
-                else
+                else if (sp >= pCur->GetParamCount())
                     nStackBase = sp - pCur->GetParamCount();
+                else
+                {
+                    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");
+                }
             }
-            if ( nStackBase > sp )
-                nStackBase = sp;        // underflow?!?
 
             switch( eOp )
             {