[Libreoffice-commits] core.git: vcl/qa vcl/source

[Caolán McNamara (via logerrit)]

 vcl/qa/cppunit/graphicfilter/data/met/fail/afl-msan-1.met |binary
 vcl/source/filter/imet/ios2met.cxx                        |   42 +++++++++++---
 2 files changed, 35 insertions(+), 7 deletions(-)

New commits:
commit 5a864ebb0e18565e33e677df7987a8f94a7742ea
Author:     Caolán McNamara <caolanm at redhat.com>
AuthorDate: Wed Sep 1 16:12:32 2021 +0100
Commit:     Caolán McNamara <caolanm at redhat.com>
CommitDate: Thu Sep 2 09:28:47 2021 +0200

    ofz: MemorySanitizer: use-of-uninitialized-value
    
    bodge dodgy SetStreamSize for now
    
    Change-Id: I4f15a2f2e196eb7b4a22331e66966f242d8968c5
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/121483
    Tested-by: Jenkins
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/vcl/qa/cppunit/graphicfilter/data/met/fail/afl-msan-1.met b/vcl/qa/cppunit/graphicfilter/data/met/fail/afl-msan-1.met
new file mode 100644
index 000000000000..18877582a4e2
Binary files /dev/null and b/vcl/qa/cppunit/graphicfilter/data/met/fail/afl-msan-1.met differ
diff --git a/vcl/source/filter/imet/ios2met.cxx b/vcl/source/filter/imet/ios2met.cxx
index 98b4c2bb82cc..774ed1517b9d 100644
--- a/vcl/source/filter/imet/ios2met.cxx
+++ b/vcl/source/filter/imet/ios2met.cxx
@@ -364,7 +364,7 @@ private:
     OSAttr   aAttr;
     OSAttr   * pAttrStack;
 
-    std::unique_ptr<SvStream> xOrdFile;
+    std::unique_ptr<SvMemoryStream> xOrdFile;
 
     void AddPointsToPath(const tools::Polygon & rPoly);
     void AddPointsToArea(const tools::Polygon & rPoly);
@@ -2560,22 +2560,48 @@ void OS2METReader::ReadField(sal_uInt16 nFieldType, sal_uInt16 nFieldSize)
         case BegGrfObjMagic:
             break;
         case EndGrfObjMagic: {
-            SvStream * pSave;
-            sal_uInt16 nOrderID, nOrderLen;
-
             if (!xOrdFile)
                 break;
 
+            auto nMaxPos = xOrdFile->Tell();
+            if (!nMaxPos)
+                break;
+
             // In xOrdFile all "DatGrfObj" fields were collected so that the
             // therein contained "Orders" are continuous and not segmented by fields.
             // To read them from the memory stream without having any trouble,
             // we use a  little trick:
 
-            pSave=pOS2MET;
+            SvStream *pSave = pOS2MET;
             pOS2MET=xOrdFile.get(); //(!)
-            auto nMaxPos = pOS2MET->Tell();
             pOS2MET->Seek(0);
 
+            // in a sane world this block is just: pOS2MET->SetStreamSize(nMaxPos);
+            if (nMaxPos)
+            {
+#ifndef NDEBUG
+                const sal_uInt8 nLastByte = static_cast<const sal_uInt8*>(xOrdFile->GetData())[nMaxPos-1];
+#endif
+                pOS2MET->SetStreamSize(nMaxPos); // shrink stream to written portion
+                assert(pOS2MET->remainingSize() == nMaxPos || pOS2MET->remainingSize() == nMaxPos - 1);
+                SAL_WARN_IF(pOS2MET->remainingSize() == nMaxPos, "filter.os2met", "this SetStreamSize workaround is no longer needed");
+                // The shrink case of SvMemoryStream::ReAllocateMemory, i.e. nEndOfData = nNewSize - 1, looks buggy to me, workaround
+                // it by using Seek to move the nEndOfData to the sane position
+                if (pOS2MET->remainingSize() < nMaxPos)
+                {
+                    pOS2MET->Seek(nMaxPos);
+                    pOS2MET->Seek(0);
+                }
+
+                assert(nLastByte == static_cast<const sal_uInt8*>(xOrdFile->GetData())[nMaxPos-1]);
+            }
+
+            assert(pOS2MET->remainingSize() == nMaxPos);
+
+            // disable stream growing past its current size
+            xOrdFile->SetResizeOffset(0);
+
+
             // "Segment header":
             sal_uInt8 nbyte(0);
             pOS2MET->ReadUChar( nbyte );
@@ -2586,11 +2612,13 @@ void OS2METReader::ReadField(sal_uInt16 nFieldType, sal_uInt16 nFieldSize)
 
             // loop through Order:
             while (pOS2MET->Tell()<nMaxPos && pOS2MET->GetError()==ERRCODE_NONE) {
-                pOS2MET->ReadUChar( nbyte ); nOrderID=static_cast<sal_uInt16>(nbyte) & 0x00ff;
+                pOS2MET->ReadUChar( nbyte );
+                sal_uInt16 nOrderID = static_cast<sal_uInt16>(nbyte) & 0x00ff;
                 if (nOrderID==0x00fe) {
                     pOS2MET->ReadUChar( nbyte );
                     nOrderID=(nOrderID << 8) | (static_cast<sal_uInt16>(nbyte) & 0x00ff);
                 }
+                sal_uInt16 nOrderLen;
                 if (nOrderID>0x00ff || nOrderID==GOrdPolygn) {
                     // ooo: As written in OS2 documentation, the order length should now
                     // be written as big endian word. (Quote: "Highorder byte precedes loworder byte").