use a read-only stream when reading graphic data

EmbeddedObjectRef::GetGraphicStream() creates a writable
SvMemoryStream, read the graphics data into it and then returns
the stream, which will be afterwards used to decode the graphics.
But if the data is broken, incorrect seeking may cause a seek
way past the buffer, and since the stream is writable, it would
be done and cause problems such as running out of memory.
The VersionCompatRead class is one such place that reads size
from the stream and then seeks based on the read data.

Add SvMemoryStream::MakeReadOnly() that will change the stream
to be read-only after the initial read of the data into it.

Change-Id: I1d44aabaf73071a691adafa489e65e4f5e3f021d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/137002
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>

3 files changed, 42 insertions, 0 deletions

diff --git a/tools/CppunitTest_tools_test.mk b/tools/CppunitTest_tools_test.mk
index cfc2a0b875fc..02a64209669a 100644
--- a/tools/CppunitTest_tools_test.mk
+++ b/tools/CppunitTest_tools_test.mk
@@ -62,6 +62,7 @@ $(eval $(call gb_CppunitTest_use_libraries,tools_test, \
     tl \
     test \
     unotest \
+    vcl \
 ))
 
 $(eval $(call gb_CppunitTest_use_static_libraries,tools_test, \
diff --git a/tools/qa/cppunit/test_stream.cxx b/tools/qa/cppunit/test_stream.cxx
index 4672af6c70c9..9a97fb5c4671 100644
--- a/tools/qa/cppunit/test_stream.cxx
+++ b/tools/qa/cppunit/test_stream.cxx
@@ -27,6 +27,7 @@ namespace
         void test_read_cstring();
         void test_read_pstring();
         void test_readline();
+        void test_makereadonly();
 
         CPPUNIT_TEST_SUITE(Test);
         CPPUNIT_TEST(test_stdstream);
@@ -34,6 +35,7 @@ namespace
         CPPUNIT_TEST(test_read_cstring);
         CPPUNIT_TEST(test_read_pstring);
         CPPUNIT_TEST(test_readline);
+        CPPUNIT_TEST(test_makereadonly);
         CPPUNIT_TEST_SUITE_END();
     };
 
@@ -275,6 +277,37 @@ namespace
         CPPUNIT_ASSERT(issB.eof());         //<-- diff A
     }
 
+    void Test::test_makereadonly()
+    {
+        SvMemoryStream aMemStream;
+        CPPUNIT_ASSERT(aMemStream.IsWritable());
+        CPPUNIT_ASSERT_EQUAL(sal_uInt64(0), aMemStream.GetSize());
+        aMemStream.WriteInt64(21);
+        CPPUNIT_ASSERT_EQUAL(ERRCODE_NONE, aMemStream.GetError());
+        CPPUNIT_ASSERT_EQUAL(sal_uInt64(8), aMemStream.GetSize());
+
+        aMemStream.MakeReadOnly();
+        CPPUNIT_ASSERT(!aMemStream.IsWritable());
+        CPPUNIT_ASSERT_EQUAL(sal_uInt64(8), aMemStream.GetSize());
+        aMemStream.WriteInt64(42);
+        CPPUNIT_ASSERT_EQUAL(ERRCODE_IO_CANTWRITE, aMemStream.GetError());
+        CPPUNIT_ASSERT_EQUAL(sal_uInt64(8), aMemStream.GetSize());
+
+        aMemStream.ResetError();
+        // Check that seeking past the end doesn't resize a read-only stream.
+        // This apparently doesn't set an error, but at least it shouldn't
+        // change the size.
+        aMemStream.Seek(1024LL*1024*1024*3);
+        CPPUNIT_ASSERT_EQUAL(sal_uInt64(8), aMemStream.GetSize());
+
+        aMemStream.ResetError();
+        aMemStream.Seek(0);
+        sal_Int64 res;
+        aMemStream.ReadInt64(res);
+        CPPUNIT_ASSERT_EQUAL(ERRCODE_NONE, aMemStream.GetError());
+        CPPUNIT_ASSERT_EQUAL(sal_Int64(21), res);
+    }
+
     CPPUNIT_TEST_SUITE_REGISTRATION(Test);
 }
 
diff --git a/tools/source/stream/stream.cxx b/tools/source/stream/stream.cxx
index 9ab7291e3bfc..fd4ce602ca99 100644
--- a/tools/source/stream/stream.cxx
+++ b/tools/source/stream/stream.cxx
@@ -1794,6 +1794,14 @@ void SvMemoryStream::SetSize(sal_uInt64 const nNewSize)
     ReAllocateMemory( nDiff );
 }
 
+void SvMemoryStream::MakeReadOnly()
+{
+    FlushBuffer();
+    m_isWritable = false;
+    nResize     = 0;
+    SetBufferSize( 0 );
+}
+
 // Create an OString of nLen bytes from rStream
 // coverity[ +taint_sanitize ]
 OString read_uInt8s_ToOString(SvStream& rStrm, std::size_t nLen)