Guard against bad stream in TypeSerializer::readGradient

Desktop / LibreOffice - Stephan Bergmann [redhat.com] - 22 October 2020 12:03 UTC

...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. failed with

> cppunittester: /home/tdf/lode/jenkins/workspace/lo_tb_master_linux_dbg/include/o3tl/strong_int.hxx:94: constexpr o3tl::strong_int::strong_int(T, typename std::enable_if::value, int>::type) [with T = short unsigned int; UNDERLYING_TYPE = short int; PHANTOM_TYPE = Degree10Tag; typename std::enable_if::value, int>::type = int]: Assertion `detail::isInRange(value) && "out of range"' failed.

apparently because nAngle happened to contain a value > std::limits::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

cb310560a887 Guard against bad stream in TypeSerializer::readGradient
vcl/source/gdi/TypeSerializer.cxx | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

Upstream: cgit.freedesktop.org


  • Share