NEWS | 1 src/lib/Makefile.am | 9 ++- src/lib/VSD5Parser.cpp | 2 src/lib/VSDInternalStream.cpp | 2 src/lib/VSDMetaData.cpp | 5 + src/lib/VSDParser.cpp | 3 + src/lib/VSDXMetaData.cpp | 4 - src/lib/libvisio_utils.cpp | 17 ++--- src/test/.gitignore | 5 + src/test/Makefile.am | 91 ++++++++++++++++++++----------- src/test/VSDInternalStreamTest.cpp | 106 +++++++++++++++++++++++++++++++++++++ 11 files changed, 195 insertions(+), 50 deletions(-)
New commits: commit b91ec4938fbe3b9d9e74b9d509ef4fdb8eb05246 Author: David Tardon <dtar...@redhat.com> Date: Sat Oct 21 12:33:00 2017 +0200 add coverity to NEWS Change-Id: I286544c96c55d81c44d52c5a6a1fdc591287b451 diff --git a/NEWS b/NEWS index 1231ebc..9a232a7 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,7 @@ libvisio 0.1.6 - Fix various crashes, leaks and hangs when reading damaged files found by oss-fuzz. - Drop outdated Windows project files. +- Fix some issues found by Coverity. - Many other small improvements and fixes. libvisio 0.1.5 commit d68dfb12140593c36526ba0bb6495300dbc974fe Author: David Tardon <dtar...@redhat.com> Date: Sat Oct 21 12:31:27 2017 +0200 cid#1312142 sanitize loop bound Change-Id: I08e9f51af44929e41bd68f3fae843105eb2c6930 diff --git a/src/lib/VSDMetaData.cpp b/src/lib/VSDMetaData.cpp index 1652193..af10ed5 100644 --- a/src/lib/VSDMetaData.cpp +++ b/src/lib/VSDMetaData.cpp @@ -127,6 +127,9 @@ void libvisio::VSDMetaData::readPropertySet(librevenge::RVNGInputStream *input, // Size input->seek(4, librevenge::RVNG_SEEK_CUR); uint32_t numProperties = readU32(input); + // The exact size of a property is not known beforehand: check upper bound + if (numProperties > getRemainingLength(input) / 12) + numProperties = getRemainingLength(input) / 12; for (uint32_t i = 0; i < numProperties; ++i) readPropertyIdentifierAndOffset(input); for (uint32_t i = 0; i < numProperties; ++i) commit ef52a208b720a6f6e3c748ab51ee19c859e9e186 Author: David Tardon <dtar...@redhat.com> Date: Sat Oct 21 12:27:35 2017 +0200 cid#1256666 sanitize loop bound Change-Id: Ib853772be55563f2ccd548f866ec299d7b906d6a diff --git a/src/lib/VSDMetaData.cpp b/src/lib/VSDMetaData.cpp index a7bcb4d..1652193 100644 --- a/src/lib/VSDMetaData.cpp +++ b/src/lib/VSDMetaData.cpp @@ -238,6 +238,8 @@ void libvisio::VSDMetaData::readTypedPropertyValue(librevenge::RVNGInputStream * librevenge::RVNGString libvisio::VSDMetaData::readCodePageString(librevenge::RVNGInputStream *input) { uint32_t size = readU32(input); + if (size > getRemainingLength(input)) + size = getRemainingLength(input); if (size == 0) return librevenge::RVNGString(); commit 99e5b63aae2e154be920edd789a4a2236af10c92 Author: David Tardon <dtar...@redhat.com> Date: Sat Oct 21 12:19:16 2017 +0200 cid#1219709 sanitize loop bound Change-Id: I199555c4b59d837af7e30cd7f8c6f643b3f23a5e diff --git a/src/lib/VSD5Parser.cpp b/src/lib/VSD5Parser.cpp index 12e5ed0..de5f85b 100644 --- a/src/lib/VSD5Parser.cpp +++ b/src/lib/VSD5Parser.cpp @@ -472,6 +472,8 @@ void libvisio::VSD5Parser::readNameIDX(librevenge::RVNGInputStream *input) VSD_DEBUG_MSG(("VSD5Parser::readNameIDX\n")); std::map<unsigned, VSDName> names; unsigned recordCount = readU16(input); + if (recordCount > getRemainingLength(input) / 4) + recordCount = getRemainingLength(input) / 4; for (unsigned i = 0; i < recordCount; ++i) { unsigned nameId = readU16(input); commit 895f7ec3c28b498b7a293a40596c24a7dcb76e30 Author: David Tardon <dtar...@redhat.com> Date: Sat Oct 21 12:15:01 2017 +0200 cid#1219708 sanitize loop bound Change-Id: I60bd5e3862ea1cca18089c0f65c40e4cc3173832 diff --git a/src/lib/VSDParser.cpp b/src/lib/VSDParser.cpp index f0a2237..69d3d56 100644 --- a/src/lib/VSDParser.cpp +++ b/src/lib/VSDParser.cpp @@ -1623,6 +1623,7 @@ void libvisio::VSDParser::readShapeData(librevenge::RVNGInputStream *input) unsigned char xType = readU8(input); unsigned char yType = readU8(input); unsigned pointCount = readU32(input); + sanitizeListLength(pointCount, 16, input); for (unsigned i = 0; i < pointCount; i++) { @@ -1647,6 +1648,7 @@ void libvisio::VSDParser::readShapeData(librevenge::RVNGInputStream *input) unsigned char xType = readU8(input); unsigned char yType = readU8(input); unsigned pointCount = readU32(input); + sanitizeListLength(pointCount, 32, input); std::vector<double> knotVector; std::vector<std::pair<double, double> > controlPoints; commit 3d563913b415e461781f6ed02a2cf1eceb3997c2 Author: David Tardon <dtar...@redhat.com> Date: Sat Oct 21 12:11:35 2017 +0200 cid#1219707 sanitize loop bound Change-Id: Idc3ffb4f82122e228f11af4ca5c08b9b988f2c98 diff --git a/src/lib/VSDParser.cpp b/src/lib/VSDParser.cpp index 31baf5a..f0a2237 100644 --- a/src/lib/VSDParser.cpp +++ b/src/lib/VSDParser.cpp @@ -824,6 +824,7 @@ void libvisio::VSDParser::readNameIDX(librevenge::RVNGInputStream *input) { std::map<unsigned, VSDName> names; unsigned recordCount = readU32(input); + sanitizeListLength(recordCount, 13, input); for (unsigned i = 0; i < recordCount; ++i) { unsigned nameId = readU32(input); commit 74ea3694d6a341c9821edd958fd647fbb0458273 Author: David Tardon <dtar...@redhat.com> Date: Sat Oct 21 12:04:49 2017 +0200 cid#1325211 fix logic Change-Id: I62fb5bec286444c4cc6099d00474bf260f4ae35b diff --git a/src/lib/VSDXMetaData.cpp b/src/lib/VSDXMetaData.cpp index 6d3826b..c0e5708 100644 --- a/src/lib/VSDXMetaData.cpp +++ b/src/lib/VSDXMetaData.cpp @@ -108,8 +108,8 @@ void libvisio::VSDXMetaData::readCoreProperties(xmlTextReaderPtr reader) break; } } - while ((XML_CP_COREPROPERTIES != tokenId || XML_READER_TYPE_END_ELEMENT != tokenType || - XML_PROPERTIES != tokenId) && 1 == ret); + while (((XML_CP_COREPROPERTIES != tokenId && XML_PROPERTIES != tokenId) || XML_READER_TYPE_END_ELEMENT != tokenType) + && 1 == ret); } bool libvisio::VSDXMetaData::parse(librevenge::RVNGInputStream *input) commit b1fe6905534d6e35ac4ef8b3d3266f1a83a772cd Author: David Tardon <dtar...@redhat.com> Date: Sat Oct 21 11:59:05 2017 +0200 cid#1419957 rewrite to avoid coverity warning Change-Id: I4c1f1e3f7011c35625c8c56afa9ddf04d1217aa3 diff --git a/src/lib/libvisio_utils.cpp b/src/lib/libvisio_utils.cpp index a46f833..63ab796 100644 --- a/src/lib/libvisio_utils.cpp +++ b/src/lib/libvisio_utils.cpp @@ -114,26 +114,21 @@ unsigned long libvisio::getRemainingLength(librevenge::RVNGInputStream *const in if (!input) throw EndOfStreamException(); - const unsigned long begin = (unsigned long) input->tell(); - unsigned long end = begin; + const long begin = input->tell(); - if (0 == input->seek(0, librevenge::RVNG_SEEK_END)) - { - end = (unsigned long) input->tell(); - } - else + if (input->seek(0, librevenge::RVNG_SEEK_END) != 0) { // librevenge::RVNG_SEEK_END does not work. Use the harder way. while (!input->isEnd()) - { readU8(input); - ++end; - } } + const long end = input->tell(); input->seek(begin, librevenge::RVNG_SEEK_SET); - return end - begin; + if (end < begin) + throw EndOfStreamException(); + return static_cast<unsigned long>(end - begin); } void libvisio::appendUCS4(librevenge::RVNGString &text, UChar32 ucs4Character) commit 61419371f3dceaa6a959a22f77f431966c9689f5 Author: David Tardon <dtar...@redhat.com> Date: Sat Oct 21 13:56:49 2017 +0200 add test for seek to end Change-Id: Iab6177acc5ae4ca78b34b4110d9c0e725d8d5d87 diff --git a/src/lib/Makefile.am b/src/lib/Makefile.am index 2828971..d8a798e 100644 --- a/src/lib/Makefile.am +++ b/src/lib/Makefile.am @@ -4,6 +4,7 @@ else version_info = -version-info $(LT_CURRENT):$(LT_REVISION):$(LT_AGE) endif +noinst_LTLIBRARIES = libvisio-internal.la lib_LTLIBRARIES = libvisio-@VSD_MAJOR_VERSION@.@VSD_MINOR_VERSION@.la AM_CXXFLAGS = \ @@ -19,10 +20,13 @@ AM_CXXFLAGS = \ BUILT_SOURCES = tokens.h tokenhash.h -libvisio_@VSD_MAJOR_VERSION@_@VSD_MINOR_VERSION@_la_LIBADD = $(LIBVISIO_LIBS) @LIBVISIO_WIN32_RESOURCE@ -libvisio_@VSD_MAJOR_VERSION@_@VSD_MINOR_VERSION@_la_DEPENDENCIES = @LIBVISIO_WIN32_RESOURCE@ +libvisio_@VSD_MAJOR_VERSION@_@VSD_MINOR_VERSION@_la_LIBADD = $(LIBVISIO_LIBS) libvisio-internal.la @LIBVISIO_WIN32_RESOURCE@ +libvisio_@VSD_MAJOR_VERSION@_@VSD_MINOR_VERSION@_la_DEPENDENCIES = libvisio-internal.la @LIBVISIO_WIN32_RESOURCE@ libvisio_@VSD_MAJOR_VERSION@_@VSD_MINOR_VERSION@_la_LDFLAGS = $(version_info) -export-dynamic -no-undefined libvisio_@VSD_MAJOR_VERSION@_@VSD_MINOR_VERSION@_la_SOURCES = \ + VisioDocument.cpp + +libvisio_internal_la_SOURCES = \ VDXParser.cpp \ VDXParser.h \ VSD5Parser.cpp \ @@ -74,7 +78,6 @@ libvisio_@VSD_MAJOR_VERSION@_@VSD_MINOR_VERSION@_la_SOURCES = \ VSDXParser.h \ VSDXTheme.cpp \ VSDXTheme.h \ - VisioDocument.cpp \ libvisio_utils.cpp \ libvisio_utils.h \ libvisio_xml.cpp \ diff --git a/src/test/.gitignore b/src/test/.gitignore index a4ecf9e..abf9c97 100644 --- a/src/test/.gitignore +++ b/src/test/.gitignore @@ -1,9 +1,10 @@ Makefile Makefile.in -test -testfileinfo +importtest +unittest .libs .deps +*.la *.lo *.log *.o diff --git a/src/test/Makefile.am b/src/test/Makefile.am index 7791453..086ea4d 100644 --- a/src/test/Makefile.am +++ b/src/test/Makefile.am @@ -1,38 +1,65 @@ -check_PROGRAMS = test - -AM_CXXFLAGS = \ - -I$(top_srcdir)/inc \ - $(LIBVISIO_CXXFLAGS) \ - $(REVENGE_STREAM_CFLAGS) \ - $(CPPUNIT_CFLAGS) \ - $(DEBUG_CXXFLAGS) - -test_CPPFLAGS = -DTDOC=\"$(top_srcdir)/src/test/data\" -test_LDADD = \ - ../lib/libvisio-@VSD_MAJOR_VERSION@.@VSD_MINOR_VERSION@.la \ - $(CPPUNIT_LIBS) \ - $(LIBVISIO_LIBS) \ - $(REVENGE_STREAM_LIBS) -test_SOURCES = \ - xmldrawinggenerator.cpp \ - xmldrawinggenerator.h \ - importtest.cpp \ - test.cpp +tests = importtest unittest + +check_PROGRAMS = $(tests) +check_LTLIBRARIES = libtest_driver.la + +libtest_driver_la_CPPFLAGS = \ + $(CPPUNIT_CFLAGS) \ + $(DEBUG_CXXFLAGS) + +libtest_driver_la_SOURCES = \ + test.cpp + +importtest_CPPFLAGS = \ + -DTDOC=\"$(top_srcdir)/src/test/data\" \ + -I$(top_srcdir)/inc \ + $(LIBVISIO_CXXFLAGS) \ + $(REVENGE_STREAM_CFLAGS) \ + $(CPPUNIT_CFLAGS) \ + $(DEBUG_CXXFLAGS) + +importtest_LDADD = \ + ../lib/libvisio-@VSD_MAJOR_VERSION@.@VSD_MINOR_VERSION@.la \ + libtest_driver.la \ + $(CPPUNIT_LIBS) \ + $(LIBVISIO_LIBS) \ + $(REVENGE_STREAM_LIBS) + +importtest_SOURCES = \ + xmldrawinggenerator.cpp \ + xmldrawinggenerator.h \ + importtest.cpp + +unittest_CPPFLAGS = \ + -I$(top_srcdir)/src/lib \ + $(LIBVISIO_CXXFLAGS) \ + $(CPPUNIT_CFLAGS) \ + $(DEBUG_CXXFLAGS) + +unittest_LDFLAGS = -L$(top_srcdir)/src/lib +unittest_LDADD = \ + $(top_builddir)/src/lib/libvisio-internal.la \ + libtest_driver.la \ + $(LIBVISIO_LIBS) \ + $(CPPUNIT_LIBS) + +unittest_SOURCES = \ + VSDInternalStreamTest.cpp EXTRA_DIST = \ - data/bgcolor.vsdx \ - data/bitmaps.vsd \ - data/bitmaps2.vsd \ - data/color-boxes.vsdx \ - data/fdo86664.vsdx \ - data/fdo86729-ms1252.vsd \ - data/fdo86729-utf8.vsd \ - data/dwg.vsd \ - data/dwg.vsdx \ - data/no-bgcolor.vsd \ - $(test_SOURCES) + data/bgcolor.vsdx \ + data/bitmaps.vsd \ + data/bitmaps2.vsd \ + data/color-boxes.vsdx \ + data/fdo86664.vsdx \ + data/fdo86729-ms1252.vsd \ + data/fdo86729-utf8.vsd \ + data/dwg.vsd \ + data/dwg.vsdx \ + data/no-bgcolor.vsd \ + $(importtest_SOURCES) # ImportTest::testVsdMetadataTitleUtf8 checks formatted date string AM_TESTS_ENVIRONMENT = TZ=UTC; export TZ; -TESTS = test +TESTS = $(tests) diff --git a/src/test/VSDInternalStreamTest.cpp b/src/test/VSDInternalStreamTest.cpp new file mode 100644 index 0000000..18afead --- /dev/null +++ b/src/test/VSDInternalStreamTest.cpp @@ -0,0 +1,106 @@ +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* + * This file is part of the libvisio project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include <algorithm> + +#include <cppunit/TestFixture.h> +#include <cppunit/extensions/HelperMacros.h> + +#include <librevenge/librevenge.h> + +#include "VSDInternalStream.h" + +namespace test +{ + +class VSDInternalStreamTest : public CPPUNIT_NS::TestFixture +{ +public: + virtual void setUp(); + virtual void tearDown(); + +private: + CPPUNIT_TEST_SUITE(VSDInternalStreamTest); + CPPUNIT_TEST(testRead); + CPPUNIT_TEST(testSeek); + CPPUNIT_TEST_SUITE_END(); + +private: + void testRead(); + void testSeek(); +}; + +void VSDInternalStreamTest::setUp() +{ +} + +void VSDInternalStreamTest::tearDown() +{ +} + +void VSDInternalStreamTest::testRead() +{ + const unsigned char data[] = "abc dee fgh"; + librevenge::RVNGBinaryData binData(data, sizeof(data)); + VSDInternalStream strm(binData.getDataStream(), binData.size()); + + CPPUNIT_ASSERT_MESSAGE("stream is already exhausted before starting to read", !strm.isEnd()); + + for (int i = 0; sizeof(data) != i; ++i) + { + unsigned long readBytes = 0; + const unsigned char *s = strm.read(1, readBytes); + + CPPUNIT_ASSERT(1 == readBytes); + CPPUNIT_ASSERT_EQUAL(data[i], s[0]); + CPPUNIT_ASSERT(((sizeof(data) - 1) == i) || !strm.isEnd()); + } + + CPPUNIT_ASSERT_MESSAGE("reading did not exhaust the stream", strm.isEnd()); + + strm.seek(0, librevenge::RVNG_SEEK_SET); + + unsigned long readBytes = 0; + const unsigned char *s = strm.read(sizeof(data), readBytes); + CPPUNIT_ASSERT(sizeof(data) == readBytes); + CPPUNIT_ASSERT(std::equal(data, data + sizeof(data), s)); +} + +void VSDInternalStreamTest::testSeek() +{ + const unsigned char data[] = "abc dee fgh"; + librevenge::RVNGBinaryData binData(data, sizeof(data)); + VSDInternalStream strm(binData.getDataStream(), binData.size()); + + strm.seek(0, librevenge::RVNG_SEEK_SET); + CPPUNIT_ASSERT(0 == strm.tell()); + strm.seek(2, librevenge::RVNG_SEEK_SET); + CPPUNIT_ASSERT(2 == strm.tell()); + + strm.seek(1, librevenge::RVNG_SEEK_CUR); + CPPUNIT_ASSERT(3 == strm.tell()); + strm.seek(-2, librevenge::RVNG_SEEK_CUR); + CPPUNIT_ASSERT(1 == strm.tell()); + + CPPUNIT_ASSERT(!strm.isEnd()); + CPPUNIT_ASSERT(0 == strm.seek(0, librevenge::RVNG_SEEK_END)); + CPPUNIT_ASSERT(strm.isEnd()); + CPPUNIT_ASSERT(sizeof(data) == strm.tell()); + CPPUNIT_ASSERT(0 != strm.seek(1, librevenge::RVNG_SEEK_END)); // cannot seek after the end + CPPUNIT_ASSERT(strm.isEnd()); + CPPUNIT_ASSERT(0 == strm.seek(-1, librevenge::RVNG_SEEK_END)); // but can seek before it + CPPUNIT_ASSERT(!strm.isEnd()); + CPPUNIT_ASSERT((sizeof(data) - 1) == strm.tell()); +} + +CPPUNIT_TEST_SUITE_REGISTRATION(VSDInternalStreamTest); + +} + +/* vim:set shiftwidth=2 softtabstop=2 expandtab: */ commit a77b8691322b469d2a35e2a287f81728bd0329a5 Author: David Tardon <dtar...@redhat.com> Date: Sat Oct 21 13:39:20 2017 +0200 implement seek to end for internal stream Change-Id: Iad3b402e5de853dd851012344682c55affa0c9de diff --git a/src/lib/VSDInternalStream.cpp b/src/lib/VSDInternalStream.cpp index 0fdcba3..9d66015 100644 --- a/src/lib/VSDInternalStream.cpp +++ b/src/lib/VSDInternalStream.cpp @@ -106,6 +106,8 @@ int VSDInternalStream::seek(long offset, librevenge::RVNG_SEEK_TYPE seekType) m_offset += offset; else if (seekType == librevenge::RVNG_SEEK_SET) m_offset = offset; + else if (seekType == librevenge::RVNG_SEEK_END) + m_offset = long(static_cast<unsigned long>(m_buffer.size())) + offset; if (m_offset < 0) { _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits