Re: Integer overflow in Calc lcl_getSingleCellAddressFromXMLString nColumn computation

[Regina Henschel]

Hi Stephan,

in regard to table:cell-range-address the following bug is relevant too:
https://bugs.documentfoundation.org/show_bug.cgi?id=131862
ODF: Remove deprecated attribute table:cell-range-address from element 



Kind regards
Regina

Stephan Bergmann schrieb am 09.03.2021 um 16:10:

On 08/03/2021 20:18, Regina Henschel wrote:
Your problem with table:cell-range-address="PivotChart" might be 
related to bug https://bugs.documentfoundation.org/show_bug.cgi?id=112783
"PIVOT CHARTS: Save produces invalid file because of invalid cell 
address"


Ah, thanks, that's a reassuring reference.  So I've created 
 "Avoid 
signed-integer-overflow parsing table:cell-range-address='PivotChart'" now.


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice



___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Integer overflow in Calc lcl_getSingleCellAddressFromXMLString nColumn computation

[Stephan Bergmann]

On 08/03/2021 20:18, Regina Henschel wrote:
Your problem with table:cell-range-address="PivotChart" might be related 
to bug https://bugs.documentfoundation.org/show_bug.cgi?id=112783

"PIVOT CHARTS: Save produces invalid file because of invalid cell address"


Ah, thanks, that's a reassuring reference.  So I've created 
 "Avoid 
signed-integer-overflow parsing table:cell-range-address='PivotChart'" now.


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Integer overflow in Calc lcl_getSingleCellAddressFromXMLString nColumn computation

[Kohei Yoshida]

On 08.03.2021 18:00, Kohei Yoshida wrote:

On 08.03.2021 11:09, Stephan Bergmann wrote:


On recent master, if I create a bare-bones pivot.ods containing a
pivot chart (in a fresh Calc document, type "A" into A1 and "2" into
A2, click into A1, then "Insert - Pivot Table... - OK" and drag "A"
from "Available Fields" to "Column Fields" and "OK", then "Insert -
Chart... - Finish", then save):  The resulting pivot.ods "Object
1/content.xml" sub-file contains a chart:plot-area XML element with a
table:cell-range-address="PivotChart" attribute as above, but which
appears to be nonsense according to the ODF standard as quoted below.

Maybe some Calc expert can shed some light on what is going on here, 
and if that


  

is legitimate, and should legitimately be processed with
lcl_getSingleCellAddressFromXMLString as is done here.


Added Tomaz to the CC list since (I believe) he has worked on pivot
chart and may have a fresher memory on this topic.


Also, since ODF v1.2 was approved in 2011, and the pivot chart feature 
was added sometime around 2017, it's possible that we are doing 
something "ahead" of the latest standard.


Kohei
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Integer overflow in Calc lcl_getSingleCellAddressFromXMLString nColumn computation

[Kohei Yoshida]

On 08.03.2021 11:09, Stephan Bergmann wrote:


On recent master, if I create a bare-bones pivot.ods containing a
pivot chart (in a fresh Calc document, type "A" into A1 and "2" into
A2, click into A1, then "Insert - Pivot Table... - OK" and drag "A"
from "Available Fields" to "Column Fields" and "OK", then "Insert -
Chart... - Finish", then save):  The resulting pivot.ods "Object
1/content.xml" sub-file contains a chart:plot-area XML element with a
table:cell-range-address="PivotChart" attribute as above, but which
appears to be nonsense according to the ODF standard as quoted below.

Maybe some Calc expert can shed some light on what is going on here, 
and if that


  

is legitimate, and should legitimately be processed with
lcl_getSingleCellAddressFromXMLString as is done here.


Added Tomaz to the CC list since (I believe) he has worked on pivot 
chart and may have a fresher memory on this topic.


I myself have hard time remembering the detail around this plot area 
data source address requirements.  I do seem to think that there was 
some weird exception that "allowed" a regular name to be used as a data 
source especially when the data source is not from Calc document but is 
from the internal data provider...  But I wouldn't trust my own memory.


Kohei
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Integer overflow in Calc lcl_getSingleCellAddressFromXMLString nColumn computation

[Regina Henschel]

Hi Stephan,

Your problem with table:cell-range-address="PivotChart" might be related 
to bug https://bugs.documentfoundation.org/show_bug.cgi?id=112783

"PIVOT CHARTS: Save produces invalid file because of invalid cell address"

Kind regards
Regina

Stephan Bergmann schrieb am 08.03.2021 um 17:09:

On 23/02/2021 11:08, Stephan Bergmann wrote:

On 23/02/2021 08:34, Stephan Bergmann wrote:
I have no idea whether lcl_getSingleCellAddressFromXMLString is 
legitimately getting called here with those arguments (or if the real 
error is somewhere else), what that nColumn computation actually 
means, nor what's going on in general.  If anybody knowledgeable 
about that code could please have a look.


With a little more digging:  However sc/qa/uitest/data/tdf107097.ods 
was generated, its "Object 1/content.xml" sub-file contains the XML 
element


table:cell-range-address="PivotChart" 
chart:data-source-has-labels="column" svg:x="0.398cm" svg:y="0.991cm" 
svg:width="16.013cm" svg:height="9.381cm">


whose table:cell-range-address attribute appears to what gets 
processed here.


On recent master, if I create a bare-bones pivot.ods containing a pivot 
chart (in a fresh Calc document, type "A" into A1 and "2" into A2, click 
into A1, then "Insert - Pivot Table... - OK" and drag "A" from 
"Available Fields" to "Column Fields" and "OK", then "Insert - Chart... 
- Finish", then save):  The resulting pivot.ods "Object 1/content.xml" 
sub-file contains a chart:plot-area XML element with a 
table:cell-range-address="PivotChart" attribute as above, but which 
appears to be nonsense according to the ODF standard as quoted below.


Maybe some Calc expert can shed some light on what is going on here, and 
if that


   

is legitimate, and should legitimately be processed with 
lcl_getSingleCellAddressFromXMLString as is done here.


Now, 
 
"19.593.6 (deprecated)" specifies that that attribute 
shall be of type 
 
"18.3.6cellRangeAddressList" aka 
 
"9.2.5Cell Range Address List".  Lacking whitespace, "PivotChart" is 
apparently a list containing a single cell range addresses or cell 
addresses, and lacking a colon, it apparently is a cell address.


 
"Referencing Table Cells" specifies the structure of such a cell address:



Cell addresses are constructed as follows:

    1)The name of the table.
    2)A dot “.” (U+002E, FULL STOP).
    3)An alphabetic value representing the column. The letter A 
represents column 1, B represents column 2, and so on. AA represents 
column 27, AB represents column 28, and so on.
    4)A numeric value representing the row. The number 1 represents 
the first row, the number 2 represents the second row, and so on.


But lcl_getCellAddressFromXMLString and 
lcl_getSingleCellAddressFromXMLString in 
chart2/source/tools/XMLRangeHelper.cxx apparently attempt to parse 
something rather different:


* lcl_getCellAddressFromXMLString supports backslash quoting;

* lcl_getCellAddressFromXMLString makes the leading table name and dot 
optional;


* lcl_getSingleCellAddressFromXMLString supports an optional "$";

* lcl_getSingleCellAddressFromXMLString supports lower-case letters in 
addition to upper-case letters for the column;


* lcl_getSingleCellAddressFromXMLString makes the numeric value 
representing the row optional.


I'm still not sure what to make of all that.  Is 
sc/qa/uitest/data/tdf107097.ods bogus and should be rejected?  Xisco, 
can you please clarify how you created that file?  Should the parsing 
code in chart2/source/tools/XMLRangeHelper.cxx be less lenient and 
reject that "PivotChart" value (or is that parsing code also used in 
situations that ask for parsing another grammar)?


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Integer overflow in Calc lcl_getSingleCellAddressFromXMLString nColumn computation

[Stephan Bergmann]

On 23/02/2021 11:08, Stephan Bergmann wrote:

On 23/02/2021 08:34, Stephan Bergmann wrote:
I have no idea whether lcl_getSingleCellAddressFromXMLString is 
legitimately getting called here with those arguments (or if the real 
error is somewhere else), what that nColumn computation actually 
means, nor what's going on in general.  If anybody knowledgeable about 
that code could please have a look.


With a little more digging:  However sc/qa/uitest/data/tdf107097.ods was 
generated, its "Object 1/content.xml" sub-file contains the XML element


table:cell-range-address="PivotChart" 
chart:data-source-has-labels="column" svg:x="0.398cm" svg:y="0.991cm" 
svg:width="16.013cm" svg:height="9.381cm">


whose table:cell-range-address attribute appears to what gets processed 
here.


On recent master, if I create a bare-bones pivot.ods containing a pivot 
chart (in a fresh Calc document, type "A" into A1 and "2" into A2, click 
into A1, then "Insert - Pivot Table... - OK" and drag "A" from 
"Available Fields" to "Column Fields" and "OK", then "Insert - Chart... 
- Finish", then save):  The resulting pivot.ods "Object 1/content.xml" 
sub-file contains a chart:plot-area XML element with a 
table:cell-range-address="PivotChart" attribute as above, but which 
appears to be nonsense according to the ODF standard as quoted below.


Maybe some Calc expert can shed some light on what is going on here, and 
if that


  

is legitimate, and should legitimately be processed with 
lcl_getSingleCellAddressFromXMLString as is done here.


Now, 
 
"19.593.6 (deprecated)" specifies that that attribute 
shall be of type 
 
"18.3.6cellRangeAddressList" aka 
 
"9.2.5Cell Range Address List".  Lacking whitespace, "PivotChart" is 
apparently a list containing a single cell range addresses or cell 
addresses, and lacking a colon, it apparently is a cell address.


 
"Referencing Table Cells" specifies the structure of such a cell address:



Cell addresses are constructed as follows:

    1)The name of the table.
    2)A dot “.” (U+002E, FULL STOP).
    3)An alphabetic value representing the column. The letter A 
represents column 1, B represents column 2, and so on. AA represents 
column 27, AB represents column 28, and so on.
    4)A numeric value representing the row. The number 1 represents 
the first row, the number 2 represents the second row, and so on.


But lcl_getCellAddressFromXMLString and 
lcl_getSingleCellAddressFromXMLString in 
chart2/source/tools/XMLRangeHelper.cxx apparently attempt to parse 
something rather different:


* lcl_getCellAddressFromXMLString supports backslash quoting;

* lcl_getCellAddressFromXMLString makes the leading table name and dot 
optional;


* lcl_getSingleCellAddressFromXMLString supports an optional "$";

* lcl_getSingleCellAddressFromXMLString supports lower-case letters in 
addition to upper-case letters for the column;


* lcl_getSingleCellAddressFromXMLString makes the numeric value 
representing the row optional.


I'm still not sure what to make of all that.  Is 
sc/qa/uitest/data/tdf107097.ods bogus and should be rejected?  Xisco, 
can you please clarify how you created that file?  Should the parsing 
code in chart2/source/tools/XMLRangeHelper.cxx be less lenient and 
reject that "PivotChart" value (or is that parsing code also used in 
situations that ask for parsing another grammar)?


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Integer overflow in Calc lcl_getSingleCellAddressFromXMLString nColumn computation

[Mike Kaganski]

On 23.02.2021 13:08, Stephan Bergmann wrote:

On 23/02/2021 08:34, Stephan Bergmann wrote:
Now, 
 
"19.593.6 (deprecated)" specifies that that attribute 
shall be of type 
 
"18.3.6cellRangeAddressList" aka 
 
"9.2.5Cell Range Address List".  Lacking whitespace, "PivotChart" is 
apparently a list containing a single cell range addresses or cell 
addresses, and lacking a colon, it apparently is a cell address.


 
"Referencing Table Cells" specifies the structure of such a cell address:



Cell addresses are constructed as follows:

    1)The name of the table.
    2)A dot “.” (U+002E, FULL STOP).
    3)An alphabetic value representing the column. The letter A 
represents column 1, B represents column 2, and so on. AA represents 
column 27, AB represents column 28, and so on.
    4)A numeric value representing the row. The number 1 represents 
the first row, the number 2 represents the second row, and so on.


But lcl_getCellAddressFromXMLString and 
lcl_getSingleCellAddressFromXMLString in 
chart2/source/tools/XMLRangeHelper.cxx apparently attempt to parse 
something rather different:


* lcl_getCellAddressFromXMLString supports backslash quoting;

* lcl_getCellAddressFromXMLString makes the leading table name and dot 
optional;


* lcl_getSingleCellAddressFromXMLString supports an optional "$";

* lcl_getSingleCellAddressFromXMLString supports lower-case letters in 
addition to upper-case letters for the column;


* lcl_getSingleCellAddressFromXMLString makes the numeric value 
representing the row optional.


I'm still not sure what to make of all that.  Is 
sc/qa/uitest/data/tdf107097.ods bogus and should be rejected?


The file has served a nice job of covering this possible problem of user 
input, and IMO should stay after the checks are fixed (but I don't have 
a suggestion on which level sanitizing should happen).




--
Best regards,
Mike Kaganski
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Integer overflow in Calc lcl_getSingleCellAddressFromXMLString nColumn computation

[Xisco Fauli]

Hi Stephan,

On 23/2/21 11:08, Stephan Bergmann wrote:
>
> I'm still not sure what to make of all that.  Is
> sc/qa/uitest/data/tdf107097.ods bogus and should be rejected?  Xisco,
> can you please clarify how you created that file?

I got it from
https://bugs.documentfoundation.org/attachment.cgi?id=132456 ( as
explained in the steps to reproduce tdf#107097 ). The original file was
attached in tdf#107074

-- 
Xisco Faulí
LibreOffice QA Team
IRC: x1sc0

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Integer overflow in Calc lcl_getSingleCellAddressFromXMLString nColumn computation

[Stephan Bergmann]

On 23/02/2021 08:34, Stephan Bergmann wrote:
I have no idea whether lcl_getSingleCellAddressFromXMLString is 
legitimately getting called here with those arguments (or if the real 
error is somewhere else), what that nColumn computation actually means, 
nor what's going on in general.  If anybody knowledgeable about that 
code could please have a look.


With a little more digging:  However sc/qa/uitest/data/tdf107097.ods was 
generated, its "Object 1/content.xml" sub-file contains the XML element






whose table:cell-range-address attribute appears to what gets processed 
here.


Now, 
 
"19.593.6 (deprecated)" specifies that that attribute 
shall be of type 
 
"18.3.6cellRangeAddressList" aka 
 
"9.2.5Cell Range Address List".  Lacking whitespace, "PivotChart" is 
apparently a list containing a single cell range addresses or cell 
addresses, and lacking a colon, it apparently is a cell address.


 
"Referencing Table Cells" specifies the structure of such a cell address:



Cell addresses are constructed as follows:

1)The name of the table. 

2)A dot “.” (U+002E, FULL STOP). 

3)An alphabetic value representing the column. The letter A represents column 1, B represents column 2, and so on. AA represents column 27, AB represents column 28, and so on. 


4)A numeric value representing the row. The number 1 represents the first 
row, the number 2 represents the second row, and so on.


But lcl_getCellAddressFromXMLString and 
lcl_getSingleCellAddressFromXMLString in 
chart2/source/tools/XMLRangeHelper.cxx apparently attempt to parse 
something rather different:


* lcl_getCellAddressFromXMLString supports backslash quoting;

* lcl_getCellAddressFromXMLString makes the leading table name and dot 
optional;


* lcl_getSingleCellAddressFromXMLString supports an optional "$";

* lcl_getSingleCellAddressFromXMLString supports lower-case letters in 
addition to upper-case letters for the column;


* lcl_getSingleCellAddressFromXMLString makes the numeric value 
representing the row optional.


I'm still not sure what to make of all that.  Is 
sc/qa/uitest/data/tdf107097.ods bogus and should be rejected?  Xisco, 
can you please clarify how you created that file?  Should the parsing 
code in chart2/source/tools/XMLRangeHelper.cxx be less lenient and 
reject that "PivotChart" value (or is that parsing code also used in 
situations that ask for parsing another grammar)?


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Integer overflow in Calc lcl_getSingleCellAddressFromXMLString nColumn computation

[Stephan Bergmann]

Ever since 
 
"tdf#107097: sc: Add UItest" added that test, UBSan builds like 
 keep failing 
UITest_chart UITEST_TEST_NAME=tdf107097.tdf107097.test_tdf107097 with



/chart2/source/tools/XMLRangeHelper.cxx:136:52: runtime error: signed integer 
overflow: 15 * 308915776 cannot be represented in type 'int'
#0 0x2ad74a554918 in (anonymous 
namespace)::lcl_getSingleCellAddressFromXMLString(rtl::OUString const&, int, int, 
chart::XMLRangeHelper::Cell&) /chart2/source/tools/XMLRangeHelper.cxx:136:52
#1 0x2ad74a553482 in (anonymous 
namespace)::lcl_getCellAddressFromXMLString(rtl::OUString const&, int, int, 
chart::XMLRangeHelper::Cell&, rtl::OUString&) 
/chart2/source/tools/XMLRangeHelper.cxx:217:13
#2 0x2ad74a5505da in (anonymous 
namespace)::lcl_getCellRangeAddressFromXMLString(rtl::OUString const&, int, int, 
chart::XMLRangeHelper::CellRange&) /chart2/source/tools/XMLRangeHelper.cxx:253:19
#3 0x2ad74a54fde1 in 
chart::XMLRangeHelper::getCellRangeFromXMLString(rtl::OUString const&) 
/chart2/source/tools/XMLRangeHelper.cxx:328:15
#4 0x2ad74a2aed4d in 
chart::InternalDataProvider::convertRangeFromXML(rtl::OUString const&) 
/chart2/source/tools/InternalDataProvider.cxx:1227:39
#5 0x2ad74a2b0164 in non-virtual thunk to 
chart::InternalDataProvider::convertRangeFromXML(rtl::OUString const&) 
/chart2/source/tools/InternalDataProvider.cxx
#6 0x2ad6c4784257 in (anonymous namespace)::lcl_ConvertRange(rtl::OUString const&, 
com::sun::star::uno::Reference const&) 
/xmloff/source/chart/SchXMLPlotAreaContext.cxx:76:32
#7 0x2ad6c4779a67 in SchXMLPlotAreaContext::startFastElement(int, 
com::sun::star::uno::Reference 
const&) /xmloff/source/chart/SchXMLPlotAreaContext.cxx:233:34
#8 0x2ad6c4c6328a in SvXMLImport::startFastElement(int, 
com::sun::star::uno::Reference 
const&) /xmloff/source/core/xmlimp.cxx:797:15
#9 0x2ad704988b78 in (anonymous namespace)::Entity::startElement((anonymous 
namespace)::Event const*) /sax/source/fastparser/fastparser.cxx:468:27
#10 0x2ad70496f681 in sax_fastparser::FastSaxParserImpl::consume((anonymous 
namespace)::EventList&) /sax/source/fastparser/fastparser.cxx:1026:25
#11 0x2ad70496c65f in 
sax_fastparser::FastSaxParserImpl::parseStream(com::sun::star::xml::sax::InputSource
 const&) /sax/source/fastparser/fastparser.cxx:870:22
#12 0x2ad7049905d1 in 
sax_fastparser::FastSaxParser::parseStream(com::sun::star::xml::sax::InputSource 
const&) /sax/source/fastparser/fastparser.cxx:1482:13
#13 0x2ad6c4c52b80 in 
SvXMLImport::parseStream(com::sun::star::xml::sax::InputSource const&) 
/xmloff/source/core/xmlimp.cxx:504:15
#14 0x2ad749aafe1e in chart::XMLFilter::impl_ImportStream(rtl::OUString const&, rtl::OUString const&, 
com::sun::star::uno::Reference const&, 
com::sun::star::uno::Reference const&, 
com::sun::star::uno::Reference const&, 
com::sun::star::uno::Reference const&) 
/chart2/source/model/filter/XMLFilter.cxx:473:34
#15 0x2ad749aa9f01 in 
chart::XMLFilter::impl_Import(com::sun::star::uno::Reference
 const&, com::sun::star::uno::Sequence const&) 
/chart2/source/model/filter/XMLFilter.cxx:375:35
#16 0x2ad749aa0988 in 
chart::XMLFilter::filter(com::sun::star::uno::Sequence
 const&) /chart2/source/model/filter/XMLFilter.cxx:221:13
#17 0x2ad749c2c76e in 
chart::ChartModel::impl_load(com::sun::star::uno::Sequence
 const&, com::sun::star::uno::Reference const&) 
/chart2/source/model/main/ChartModel_Persistence.cxx:567:18
#18 0x2ad749c30eea in 
chart::ChartModel::loadFromStorage(com::sun::star::uno::Reference
 const&, com::sun::star::uno::Sequence const&) 
/chart2/source/model/main/ChartModel_Persistence.cxx:759:5
#19 0x2ad74244b977 in OCommonEmbeddedObject::LoadDocumentFromStorage_Impl() 
/embeddedobj/source/commonembedding/persistence.cxx:535:19
#20 0x2ad7423d7bde in OCommonEmbeddedObject::SwitchStateTo_Impl(int) 
/embeddedobj/source/commonembedding/embedobj.cxx:185:49
#21 0x2ad7423e32ff in OCommonEmbeddedObject::changeState(int) 
/embeddedobj/source/commonembedding/embedobj.cxx:453:13
#22 0x2ad7424b7057 in 
OCommonEmbeddedObject::getPreferredVisualRepresentation(long) 
/embeddedobj/source/commonembedding/visobj.cxx:168:9
#23 0x2ad67e08fdb6 in 
comphelper::EmbeddedObjectContainer::GetGraphicReplacementStream(long, 
com::sun::star::uno::Reference const&, 
rtl::OUString*) /comphelper/source/container/embeddedobjectcontainer.cxx:1425:54
#24 0x2ad6a447182c in svt::EmbeddedObjectRef::GetGraphicReplacementStream(long, 
com::sun::star::uno::Reference const&, 
rtl::OUString*) /svtools/source/misc/embedhlp.cxx:809:12
#25 0x2ad6a446c7d4 in svt::EmbeddedObjectRef::GetGraphicStream(bool) const 
/svtools/source/misc/embedhlp.cxx:616:23
#26 0x2ad6a4469e58 in svt::EmbeddedObjectRef::GetReplacement(bool) 
/svtools/source/misc/embedhlp.cxx:424:46
#27 0x2ad6a446d4ea in