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.lunak@collabora.com>
diff --git a/sc/qa/unit/range.cxx b/sc/qa/unit/range.cxx
index d704eb2..843ecdd 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 668c08b..28f1b0b 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;
}
