vcl/source/gdi/TypeSerializer.cxx | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
New commits:
commit cb310560a887ba08ea4234ea6cdd217151ca0728
Author: Stephan Bergmann <sberg...@redhat.com>
AuthorDate: Thu Oct 22 11:05:07 2020 +0200
Commit: Stephan Bergmann <sberg...@redhat.com>
CommitDate: Thu Oct 22 14:03:53 2020 +0200
Guard against bad stream in TypeSerializer::readGradient
...by defaulting to zero any values that fail to get read, in line with how
other TypeSerializer::read* (vcl/source/gdi/TypeSerializer.cxx) and
underlying
GenericTypeSerializer::read* (tools/source/stream/GenericTypeSerializer.cxx)
functions handle bad streams (though TypeSerializer::readGraphic does check
mrStream's state, as a notable exception).
I observed such failed reads with `VALGRIND=memcheck make
CppunitTest_vcl_filters_test CPPUNIT_TEST_NAME=VclFiltersTest::testCVEs`:
> Conditional jump or move depends on uninitialised value(s)
> at 0x170EA438: TypeSerializer::readGradient(Gradient&)
(/vcl/source/gdi/TypeSerializer.cxx:61)
> by 0x16F00CEE: MetaFloatTransparentAction::Read(SvStream&,
ImplMetaReadData*) (/vcl/source/gdi/metaact.cxx:3020)
> by 0x16EF1704: MetaAction::ReadMetaAction(SvStream&,
ImplMetaReadData*) (/vcl/source/gdi/metaact.cxx:269)
> by 0x16E99C03: ReadGDIMetaFile(SvStream&, GDIMetaFile&,
ImplMetaReadData*) (/vcl/source/gdi/gdimtf.cxx:2693)
> by 0x16F00C91: MetaFloatTransparentAction::Read(SvStream&,
ImplMetaReadData*) (/vcl/source/gdi/metaact.cxx:3016)
> by 0x16EF1704: MetaAction::ReadMetaAction(SvStream&,
ImplMetaReadData*) (/vcl/source/gdi/metaact.cxx:269)
> by 0x16E99C03: ReadGDIMetaFile(SvStream&, GDIMetaFile&,
ImplMetaReadData*) (/vcl/source/gdi/gdimtf.cxx:2693)
> by 0x16F00C91: MetaFloatTransparentAction::Read(SvStream&,
ImplMetaReadData*) (/vcl/source/gdi/metaact.cxx:3016)
> by 0x16EF1704: MetaAction::ReadMetaAction(SvStream&,
ImplMetaReadData*) (/vcl/source/gdi/metaact.cxx:269)
> by 0x16E99C03: ReadGDIMetaFile(SvStream&, GDIMetaFile&,
ImplMetaReadData*) (/vcl/source/gdi/gdimtf.cxx:2693)
> Uninitialised value was created by a stack allocation
> at 0x170EA274: TypeSerializer::readGradient(Gradient&)
(/vcl/source/gdi/TypeSerializer.cxx:33)
(The uninitialized
sal_uInt16 nAngle
had started to cause issues since 0fb58a1ff168ae122e9c8993a3136658e3b0e3f0
"new
tools::Degree10 strong typedef", when e.g.
<https://ci.libreoffice.org/job/lo_tb_master_linux_dbg/30821/> failed with
> cppunittester:
/home/tdf/lode/jenkins/workspace/lo_tb_master_linux_dbg/include/o3tl/strong_int.hxx:94:
constexpr o3tl::strong_int<UNDERLYING_TYPE, PHANTOM_TYPE>::strong_int(T,
typename std::enable_if<std::is_integral<T>::value, int>::type) [with T = short
unsigned int; UNDERLYING_TYPE = short int; PHANTOM_TYPE = Degree10Tag; typename
std::enable_if<std::is_integral<T>::value, int>::type = int]: Assertion
`detail::isInRange<UNDERLYING_TYPE>(value) && "out of range"' failed.
apparently because nAngle happened to contain a value >
std::limits<sal_Int32>::max(), so converting it to a tools::Degree10 based
on
sal_Int16 triggered the assert. 0e8e352cc78af9b8cee27a77b0ac8e2e8f98f8cc
"clamp
angle to valid value" tried to address the symptoms, but failed to identify
the
root cause which is now addressed in this commit. I'm not sure whether that
clamping of nAngle from 0e8e352cc78af9b8cee27a77b0ac8e2e8f98f8cc is useful
in
itself, or whether nAngle should rather be of type sal_Int16 (or converted
to
such prior to being converted to Degree10). But that question of what input
values (if they are actually read in from a non-broken stream) are invalid
and
need treatment is somewhat orthogonal to this fix, so I leave that as it
currently is for now.)
Change-Id: Iae74bad9e2543bf7ac8712adbf360d5e1076bdf7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/104650
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sberg...@redhat.com>
diff --git a/vcl/source/gdi/TypeSerializer.cxx
b/vcl/source/gdi/TypeSerializer.cxx
index bbfdbf430160..c86498485d4f 100644
--- a/vcl/source/gdi/TypeSerializer.cxx
+++ b/vcl/source/gdi/TypeSerializer.cxx
@@ -33,16 +33,16 @@ void TypeSerializer::readGradient(Gradient& rGradient)
{
VersionCompat aCompat(mrStream, StreamMode::READ);
- sal_uInt16 nStyle;
+ sal_uInt16 nStyle = 0;
Color aStartColor;
Color aEndColor;
- sal_uInt16 nAngle;
- sal_uInt16 nBorder;
- sal_uInt16 nOffsetX;
- sal_uInt16 nOffsetY;
- sal_uInt16 nIntensityStart;
- sal_uInt16 nIntensityEnd;
- sal_uInt16 nStepCount;
+ sal_uInt16 nAngle = 0;
+ sal_uInt16 nBorder = 0;
+ sal_uInt16 nOffsetX = 0;
+ sal_uInt16 nOffsetY = 0;
+ sal_uInt16 nIntensityStart = 0;
+ sal_uInt16 nIntensityEnd = 0;
+ sal_uInt16 nStepCount = 0;
mrStream.ReadUInt16(nStyle);
readColor(aStartColor);
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
