sc/qa/unit/range.cxx | 11 ++++++
sc/source/core/tool/address.cxx | 63 +++++++++++++++++++---------------------
2 files changed, 42 insertions(+), 32 deletions(-)
New commits:
commit 7bb0b11873d0c96d8116b327987834db048754c3
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Thu Feb 17 14:44:48 2022 +0100
Commit: Luboš Luňák <l.lu...@collabora.com>
CommitDate: Thu Feb 17 16:59:48 2022 +0100
fix range checking when parsing Calc cell address (tdf#147451)
The document contains 'Sheet1', which Calc first tried to parse
as a normal address, since it matches the format of e.g. 'XFD1'.
The code parsed column into SCCOL (sal_Int16), which with 16k
column limit overflowed and the code failed to detect the problem.
Change-Id: I470db1b670dbff7bdc8013bede0a0b011e88c372
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/130073
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lu...@collabora.com>
diff --git a/sc/qa/unit/range.cxx b/sc/qa/unit/range.cxx
index d704eb2dc4e2..843ecddcb596 100644
--- a/sc/qa/unit/range.cxx
+++ b/sc/qa/unit/range.cxx
@@ -28,9 +28,11 @@ public:
CPPUNIT_TEST_SUITE(ScAddressTest);
CPPUNIT_TEST(testAddressParsing);
+ CPPUNIT_TEST(testTdf147451);
CPPUNIT_TEST_SUITE_END();
void testAddressParsing();
+ void testTdf147451();
private:
ScDocShellRef m_xDocShRef;
@@ -44,6 +46,15 @@ void ScAddressTest::testAddressParsing()
CPPUNIT_ASSERT_MESSAGE("Should fail to parse.", !(nRes &
ScRefFlags::VALID));
}
+void ScAddressTest::testTdf147451()
+{
+ ScAddress aAddr;
+ ScDocument& rDoc = m_xDocShRef->GetDocument();
+ // "Sheet1" is technically a valid address like "XF1", but it should
overflow.
+ ScRefFlags nRes = aAddr.Parse("Sheet1", rDoc,
formula::FormulaGrammar::CONV_OOO);
+ CPPUNIT_ASSERT_MESSAGE("Should fail to parse.", !(nRes &
ScRefFlags::VALID));
+}
+
void ScAddressTest::setUp()
{
BootstrapFixture::setUp();
diff --git a/sc/source/core/tool/address.cxx b/sc/source/core/tool/address.cxx
index 668c08b063d3..28f1b0b7f6ad 100644
--- a/sc/source/core/tool/address.cxx
+++ b/sc/source/core/tool/address.cxx
@@ -133,9 +133,9 @@ const sal_Unicode* parseQuotedName( const sal_Unicode* p,
OUString& rName )
}
-static tools::Long sal_Unicode_strtol ( const sal_Unicode* p, const
sal_Unicode** pEnd )
+static sal_Int64 sal_Unicode_strtol ( const sal_Unicode* p, const
sal_Unicode** pEnd )
{
- tools::Long accum = 0, prev = 0;
+ sal_Int64 accum = 0, prev = 0;
bool is_neg = false;
if( *p == '-' )
@@ -655,12 +655,13 @@ const sal_Unicode* ScRange::Parse_XL_Header(
return p;
}
-static const sal_Unicode* lcl_r1c1_get_col( const sal_Unicode* p,
+static const sal_Unicode* lcl_r1c1_get_col( const ScSheetLimits& rSheetLimits,
+ const sal_Unicode* p,
const ScAddress::Details& rDetails,
ScAddress* pAddr, ScRefFlags*
nFlags )
{
const sal_Unicode *pEnd;
- tools::Long n;
+ sal_Int64 n;
bool isRelative;
if( p[0] == '\0' )
@@ -693,7 +694,7 @@ static const sal_Unicode* lcl_r1c1_get_col( const
sal_Unicode* p,
n--;
}
- if( n < 0 || n >= MAXCOLCOUNT )
+ if( n < 0 || n >= rSheetLimits.GetMaxColCount())
return nullptr;
pAddr->SetCol( static_cast<SCCOL>( n ) );
*nFlags |= ScRefFlags::COL_VALID;
@@ -708,7 +709,6 @@ static const sal_Unicode* lcl_r1c1_get_row(
ScAddress* pAddr, ScRefFlags* nFlags )
{
const sal_Unicode *pEnd;
- tools::Long n;
bool isRelative;
if( p[0] == '\0' )
@@ -718,7 +718,7 @@ static const sal_Unicode* lcl_r1c1_get_row(
isRelative = *p == '[';
if( isRelative )
p++;
- n = sal_Unicode_strtol( p, &pEnd );
+ sal_Int64 n = sal_Unicode_strtol( p, &pEnd );
if( nullptr == pEnd )
return nullptr;
@@ -821,7 +821,7 @@ static ScRefFlags lcl_ScRange_Parse_XL_R1C1( ScRange& r,
return bOnlyAcceptSingle ? ScRefFlags::ZERO : nFlags;
}
- else if( nullptr == (p = lcl_r1c1_get_col( p, rDetails, &r.aStart,
&nFlags )))
+ else if( nullptr == (p = lcl_r1c1_get_col( rDoc.GetSheetLimits(), p,
rDetails, &r.aStart, &nFlags )))
{
return ScRefFlags::ZERO;
}
@@ -830,7 +830,7 @@ static ScRefFlags lcl_ScRange_Parse_XL_R1C1( ScRange& r,
(p[1] != 'R' && p[1] != 'r') ||
nullptr == (pTmp = lcl_r1c1_get_row( rDoc.GetSheetLimits(), p+1,
rDetails, &r.aEnd, &nFlags2 )) ||
(*pTmp != 'C' && *pTmp != 'c') ||
- nullptr == (pTmp = lcl_r1c1_get_col( pTmp, rDetails, &r.aEnd,
&nFlags2 )))
+ nullptr == (pTmp = lcl_r1c1_get_col( rDoc.GetSheetLimits(), pTmp,
rDetails, &r.aEnd, &nFlags2 )))
{
// single cell reference
@@ -861,11 +861,11 @@ static ScRefFlags lcl_ScRange_Parse_XL_R1C1( ScRange& r,
}
else if( *p == 'C' || *p == 'c' ) // full col C#
{
- if( nullptr == (p = lcl_r1c1_get_col( p, rDetails, &r.aStart, &nFlags
)))
+ if( nullptr == (p = lcl_r1c1_get_col( rDoc.GetSheetLimits(), p,
rDetails, &r.aStart, &nFlags )))
return nBailOutFlags;
if( p[0] != ':' || (p[1] != 'C' && p[1] != 'c') ||
- nullptr == (pTmp = lcl_r1c1_get_col( p+1, rDetails, &r.aEnd,
&nFlags2 )))
+ nullptr == (pTmp = lcl_r1c1_get_col( rDoc.GetSheetLimits(), p+1,
rDetails, &r.aEnd, &nFlags2 )))
{ // Fallback to just the initial C
applyStartToEndFlags(nFlags);
r.aEnd.SetCol( r.aStart.Col() );
@@ -902,8 +902,6 @@ static const sal_Unicode* lcl_a1_get_col( const ScDocument&
rDoc,
ScRefFlags* nFlags,
const OUString* pErrRef )
{
- SCCOL nCol;
-
if( *p == '$' )
{
*nFlags |= ScRefFlags::COL_ABS;
@@ -921,15 +919,15 @@ static const sal_Unicode* lcl_a1_get_col( const
ScDocument& rDoc,
if( !rtl::isAsciiAlpha( *p ) )
return nullptr;
- nCol = sal::static_int_cast<SCCOL>( rtl::toAsciiUpperCase( *p++ ) - 'A' );
+ sal_Int64 nCol = rtl::toAsciiUpperCase( *p++ ) - 'A';
const SCCOL nMaxCol = rDoc.MaxCol();
while (nCol <= nMaxCol && rtl::isAsciiAlpha(*p))
- nCol = sal::static_int_cast<SCCOL>( ((nCol + 1) * 26) +
rtl::toAsciiUpperCase( *p++ ) - 'A' );
- if( nCol > nMaxCol || rtl::isAsciiAlpha( *p ) )
+ nCol = ((nCol + 1) * 26) + rtl::toAsciiUpperCase( *p++ ) - 'A';
+ if( nCol > nMaxCol || nCol < 0 || rtl::isAsciiAlpha( *p ) )
return nullptr;
*nFlags |= ScRefFlags::COL_VALID;
- pAddr->SetCol( nCol );
+ pAddr->SetCol( sal::static_int_cast<SCCOL>( nCol ));
return p;
}
@@ -941,7 +939,6 @@ static const sal_Unicode* lcl_a1_get_row( const ScDocument&
rDoc,
const OUString* pErrRef )
{
const sal_Unicode *pEnd;
- tools::Long n;
if( *p == '$' )
{
@@ -957,12 +954,12 @@ static const sal_Unicode* lcl_a1_get_row( const
ScDocument& rDoc,
return p;
}
- n = sal_Unicode_strtol( p, &pEnd ) - 1;
+ sal_Int64 n = sal_Unicode_strtol( p, &pEnd ) - 1;
if( nullptr == pEnd || p == pEnd || n < 0 || n > rDoc.MaxRow() )
return nullptr;
*nFlags |= ScRefFlags::ROW_VALID;
- pAddr->SetRow( static_cast<SCROW>(n) );
+ pAddr->SetRow( sal::static_int_cast<SCROW>(n) );
return pEnd;
}
@@ -1280,19 +1277,21 @@ static ScRefFlags lcl_ScAddress_Parse_OOo( const
sal_Unicode* p, const ScDocumen
}
else
{
- const SCCOL nMaxCol = rDoc.MaxCol();
if (rtl::isAsciiAlpha( *p ))
{
- nCol = sal::static_int_cast<SCCOL>( rtl::toAsciiUpperCase(
*p++ ) - 'A' );
- while (nCol < nMaxCol && rtl::isAsciiAlpha(*p))
- nCol = sal::static_int_cast<SCCOL>( ((nCol + 1) * 26) +
rtl::toAsciiUpperCase( *p++ ) - 'A' );
+ const SCCOL nMaxCol = rDoc.MaxCol();
+ sal_Int64 n = rtl::toAsciiUpperCase( *p++ ) - 'A';
+ while (n < nMaxCol && rtl::isAsciiAlpha(*p))
+ n = ((n + 1) * 26) + rtl::toAsciiUpperCase( *p++ ) - 'A';
+ if (n > nMaxCol || n < 0 || (*p && *p != '$' &&
!rtl::isAsciiDigit( *p ) &&
+ (!pErrRef || !lcl_isString( p, *pErrRef))))
+ nBits = ScRefFlags::ZERO;
+ else
+ nCol = sal::static_int_cast<SCCOL>( n );
}
else
nBits = ScRefFlags::ZERO;
- if (nCol > nMaxCol || (*p && *p != '$' && !rtl::isAsciiDigit( *p )
&&
- (!pErrRef || !lcl_isString( p, *pErrRef))))
- nBits = ScRefFlags::ZERO;
if( nBits == ScRefFlags::ZERO )
p = q;
}
@@ -1327,17 +1326,17 @@ static ScRefFlags lcl_ScAddress_Parse_OOo( const
sal_Unicode* p, const ScDocumen
if( !rtl::isAsciiDigit( *p ) )
{
nBits = ScRefFlags::ZERO;
- nRow = SCROW(-1);
+ nRow = -1;
}
else
{
- tools::Long n = rtl_ustr_toInt32( p, 10 ) - 1;
+ sal_Int64 n = rtl_ustr_toInt32( p, 10 ) - 1;
while (rtl::isAsciiDigit( *p ))
p++;
const SCROW nMaxRow = rDoc.MaxRow();
if( n < 0 || n > nMaxRow )
nBits = ScRefFlags::ZERO;
- nRow = static_cast<SCROW>(n);
+ nRow = sal::static_int_cast<SCROW>(n);
}
if( nBits == ScRefFlags::ZERO )
p = q;
@@ -1795,12 +1794,12 @@ ScRefFlags ScRange::ParseCols( const ScDocument& rDoc,
case formula::FormulaGrammar::CONV_XL_R1C1:
if ((p[0] == 'C' || p[0] == 'c') &&
- nullptr != (p = lcl_r1c1_get_col( p, rDetails, &aStart, &ignored
)))
+ nullptr != (p = lcl_r1c1_get_col( rDoc.GetSheetLimits(), p,
rDetails, &aStart, &ignored )))
{
if( p[0] == ':')
{
if( (p[1] == 'C' || p[1] == 'c') &&
- nullptr != (p = lcl_r1c1_get_col( p+1, rDetails, &aEnd,
&ignored )))
+ nullptr != (p = lcl_r1c1_get_col( rDoc.GetSheetLimits(),
p+1, rDetails, &aEnd, &ignored )))
{
nRes = ScRefFlags::COL_VALID;
}
