ofz#3154 check bounds of special sprm

Change-Id: I82566e2f2ad479c392f06ae7149e3781c0338e50

ofz: sanity check L_VAR2 record bounds

Change-Id: I862457a7239108974f360a87b4f6ccf433eae364
Reviewed-on: https://gerrit.libreoffice.org/37534
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>
(cherry picked from commit 016e4d0e2650b2fb350068d86e8d392a7ef5acb1)

ofz: stay within available data

Change-Id: Ic959cf5b2cd92ba5bc297e686beb1fd50427a994
Reviewed-on: https://gerrit.libreoffice.org/36102
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>
(cherry picked from commit fdcac49119d3fc9f6216af834e7afc56d2c2e376)
Reviewed-on: https://gerrit.libreoffice.org/41648
Reviewed-by: Michael Stahl <mstahl@redhat.com>
(cherry picked from commit f33a136dc6bcb8bc0ed6ddd6c3d38d75e067e6eb)

8 files changed, 52 insertions, 29 deletions

diff --git a/sw/source/filter/ww8/ww8graf.cxx b/sw/source/filter/ww8/ww8graf.cxx
index bd1a8ca64cad..2664cdb3971e 100644
--- a/sw/source/filter/ww8/ww8graf.cxx
+++ b/sw/source/filter/ww8/ww8graf.cxx
@@ -617,7 +617,7 @@ void SwWW8ImplReader::InsertAttrsAsDrawingAttrs(long nStartCp, long nEndCp,
                 // off and convert them later
                 if (bStartAttr)
                 {
-                    ImportSprm(aRes.pMemPos, aRes.nSprmId);
+                    ImportSprm(aRes.pMemPos, aRes.nMemLen, aRes.nSprmId);
                     if (!bDoingSymbol && m_bSymbol)
                     {
                         bDoingSymbol = true;
diff --git a/sw/source/filter/ww8/ww8par.cxx b/sw/source/filter/ww8/ww8par.cxx
index 503ed16c6d5f..88f4f4f3d22c 100644
--- a/sw/source/filter/ww8/ww8par.cxx
+++ b/sw/source/filter/ww8/ww8par.cxx
@@ -3824,7 +3824,7 @@ long SwWW8ImplReader::ReadTextAttr(WW8_CP& rTextPos, long nTextEnd, bool& rbStar
             if( bStartAttr ) // WW attributes
             {
                 if( aRes.nMemLen >= 0 )
-                    ImportSprm(aRes.pMemPos, aRes.nSprmId);
+                    ImportSprm(aRes.pMemPos, aRes.nMemLen, aRes.nSprmId);
             }
             else
                 EndSprm( aRes.nSprmId ); // Switch off Attr
@@ -6434,7 +6434,7 @@ bool SwMSDffManager::GetOLEStorageName(long nOLEId, OUString& rStorageName,
                         while (nLen >= 2 && !nPictureId)
                         {
                             sal_uInt16 nId = aSprmParser.GetSprmId(pSprm);
-                            sal_uInt16 nSL = aSprmParser.GetSprmSize(nId, pSprm);
+                            sal_uInt16 nSL = aSprmParser.GetSprmSize(nId, pSprm, nLen);
 
                             if( nLen < nSL )
                                 break; // Not enough Bytes left
diff --git a/sw/source/filter/ww8/ww8par.hxx b/sw/source/filter/ww8/ww8par.hxx
index e117ee980422..600cb409b263 100644
--- a/sw/source/filter/ww8/ww8par.hxx
+++ b/sw/source/filter/ww8/ww8par.hxx
@@ -1846,7 +1846,7 @@ public:     // really private, but can only be done public
 
     void DeleteFormImpl();
 
-    short ImportSprm( const sal_uInt8* pPos, sal_uInt16 nId = 0 );
+    short ImportSprm(const sal_uInt8* pPos, sal_Int32 nMemLen, sal_uInt16 nId = 0);
 
     bool SearchRowEnd(WW8PLCFx_Cp_FKP* pPap,WW8_CP &rStartCp, int nLevel) const;
     /// Seek to the end of the table with pPap, returns true on success.
diff --git a/sw/source/filter/ww8/ww8par2.cxx b/sw/source/filter/ww8/ww8par2.cxx
index 869e6e86c4d1..7a3f269bb1ab 100644
--- a/sw/source/filter/ww8/ww8par2.cxx
+++ b/sw/source/filter/ww8/ww8par2.cxx
@@ -3718,7 +3718,7 @@ void WW8RStyle::ImportSprms(sal_uInt8 *pSprms, short nLen, bool bPap)
 #ifdef DEBUGSPRMREADER
         fprintf(stderr, "id is %x\n", aIter.GetAktId());
 #endif
-        pIo->ImportSprm(pSprm);
+        pIo->ImportSprm(pSprm, aSprmIter.GetRemLen(), aSprmIter.GetAktId());
         aSprmIter.advance();
     }
 
diff --git a/sw/source/filter/ww8/ww8par3.cxx b/sw/source/filter/ww8/ww8par3.cxx
index f3211bac3b6b..7b1c37c76bdf 100644
--- a/sw/source/filter/ww8/ww8par3.cxx
+++ b/sw/source/filter/ww8/ww8par3.cxx
@@ -714,7 +714,7 @@ bool WW8ListManager::ReadLVL(SwNumFormat& rNumFormat, SfxItemSet*& rpItemSet,
             maSprmParser);
         while (const sal_uInt8* pSprm = aSprmIter.GetSprms())
         {
-            rReader.ImportSprm(pSprm);
+            rReader.ImportSprm(pSprm, aSprmIter.GetRemLen(), aSprmIter.GetAktId());
             aSprmIter.advance();
         }
 
@@ -1908,7 +1908,7 @@ void SwWW8ImplReader::RegisterNumFormatOnTextNode(sal_uInt16 nActLFO,
                     sal_uInt8* pSprms1  = &aParaSprms[0];
                     while (0 < nLen)
                     {
-                        sal_uInt16 nL1 = ImportSprm(pSprms1);
+                        sal_uInt16 nL1 = ImportSprm(pSprms1, nLen);
                         nLen = nLen - nL1;
                         pSprms1 += nL1;
                     }
diff --git a/sw/source/filter/ww8/ww8par6.cxx b/sw/source/filter/ww8/ww8par6.cxx
index 5cd9e4ba74fd..f86eb4c80819 100644
--- a/sw/source/filter/ww8/ww8par6.cxx
+++ b/sw/source/filter/ww8/ww8par6.cxx
@@ -6301,7 +6301,7 @@ void SwWW8ImplReader::EndSprm( sal_uInt16 nId )
         (this->*rSprm.pReadFnc)( nId, nullptr, -1 );
 }
 
-short SwWW8ImplReader::ImportSprm(const sal_uInt8* pPos,sal_uInt16 nId)
+short SwWW8ImplReader::ImportSprm(const sal_uInt8* pPos, sal_Int32 nMemLen, sal_uInt16 nId)
 {
     if (!nId)
         nId = m_pSprmParser->GetSprmId(pPos);
@@ -6311,7 +6311,7 @@ short SwWW8ImplReader::ImportSprm(const sal_uInt8* pPos,sal_uInt16 nId)
     const SprmReadInfo& rSprm = GetSprmReadInfo(nId);
 
     sal_uInt16 nFixedLen = m_pSprmParser->DistanceToData(nId);
-    sal_uInt16 nL = m_pSprmParser->GetSprmSize(nId, pPos);
+    sal_uInt16 nL = m_pSprmParser->GetSprmSize(nId, pPos, nMemLen);
 
     if (rSprm.pReadFnc)
         (this->*rSprm.pReadFnc)(nId, pPos + nFixedLen, nL - nFixedLen);
diff --git a/sw/source/filter/ww8/ww8scan.cxx b/sw/source/filter/ww8/ww8scan.cxx
index 1d51c27a5623..4d138aa6f5f7 100644
--- a/sw/source/filter/ww8/ww8scan.cxx
+++ b/sw/source/filter/ww8/ww8scan.cxx
@@ -883,14 +883,14 @@ inline long Get_Long( sal_uInt8 *& p )
     return Get_ULong(p);
 }
 
-WW8SprmIter::WW8SprmIter(const sal_uInt8* pSprms_, long nLen_,
+WW8SprmIter::WW8SprmIter(const sal_uInt8* pSprms_, sal_Int32 nLen_,
     const wwSprmParser &rParser)
     :  mrSprmParser(rParser), pSprms( pSprms_), nRemLen( nLen_)
 {
     UpdateMyMembers();
 }
 
-void WW8SprmIter::SetSprms(const sal_uInt8* pSprms_, long nLen_)
+void WW8SprmIter::SetSprms(const sal_uInt8* pSprms_, sal_Int32 nLen_)
 {
     pSprms = pSprms_;
     nRemLen = nLen_;
@@ -917,7 +917,7 @@ void WW8SprmIter::UpdateMyMembers()
     if (bValid)
     {
         nAktId = mrSprmParser.GetSprmId(pSprms);
-        nAktSize = mrSprmParser.GetSprmSize(nAktId, pSprms);
+        nAktSize = mrSprmParser.GetSprmSize(nAktId, pSprms, nRemLen);
         pAktParams = pSprms + mrSprmParser.DistanceToData(nAktId);
         bValid = nAktSize <= nRemLen;
         SAL_WARN_IF(!bValid, "sw.ww8", "sprm longer than remaining bytes, doc or parser is wrong");
@@ -3527,7 +3527,7 @@ bool WW8PLCFx_SEPX::Find4Sprms(sal_uInt16 nId1,sal_uInt16 nId2,sal_uInt16 nId3,s
             bOk = false;
         bFound |= bOk;
         // increment pointer so that it points to next SPRM
-        const sal_uInt16 x = maSprmParser.GetSprmSize(nAktId, pSp);
+        const sal_uInt16 x = maSprmParser.GetSprmSize(nAktId, pSp, nSprmSiz - i);
         i += x;
         pSp += x;
     }
@@ -3553,7 +3553,7 @@ const sal_uInt8* WW8PLCFx_SEPX::HasSprm( sal_uInt16 nId, sal_uInt8 n2nd ) const
                 return pRet;
         }
         // increment pointer so that it points to next SPRM
-        const sal_uInt16 x = maSprmParser.GetSprmSize(nAktId, pSp);
+        const sal_uInt16 x = maSprmParser.GetSprmSize(nAktId, pSp, nSprmSiz - i);
         i += x;
         pSp += x;
     }
@@ -4901,7 +4901,7 @@ void WW8PLCFMan::GetSprmStart( short nIdx, WW8PLCFManResult* pRes ) const
     else if (p->nSprmsLen >= maSprmParser.MinSprmLen()) //normal
     {
         // Length of actual sprm
-        pRes->nMemLen = maSprmParser.GetSprmSize(pRes->nSprmId, pRes->pMemPos);
+        pRes->nMemLen = maSprmParser.GetSprmSize(pRes->nSprmId, pRes->pMemPos, p->nSprmsLen);
         if (pRes->nMemLen > p->nSprmsLen)
         {
             SAL_WARN("sw.ww8", "Short sprm, len " << pRes->nMemLen << " claimed, max possible is " << p->nSprmsLen);
@@ -5009,7 +5009,7 @@ void WW8PLCFMan::AdvSprm(short nIdx, bool bStart)
             if( p->pMemPos )
             {
                 // Length of last sprm
-                const sal_uInt16 nSprmL = maSprmParser.GetSprmSize(nLastId, p->pMemPos);
+                const sal_uInt16 nSprmL = maSprmParser.GetSprmSize(nLastId, p->pMemPos, p->nSprmsLen);
 
                 // Reduce length of all sprms by length of last sprm
                 p->nSprmsLen -= nSprmL;
@@ -7903,7 +7903,7 @@ sal_uInt16 WW8DopTypography::GetConvertedLang() const
 
 //              Sprms
 
-sal_uInt16 wwSprmParser::GetSprmTailLen(sal_uInt16 nId, const sal_uInt8* pSprm)
+sal_uInt16 wwSprmParser::GetSprmTailLen(sal_uInt16 nId, const sal_uInt8* pSprm, sal_Int32 nRemLen)
     const
 {
     SprmInfo aSprm = GetSprmInfo(nId);
@@ -7918,15 +7918,26 @@ sal_uInt16 wwSprmParser::GetSprmTailLen(sal_uInt16 nId, const sal_uInt8* pSprm)
                 nL = static_cast< sal_uInt16 >(pSprm[1 + mnDelta] + aSprm.nLen);
             else
             {
-                sal_uInt8 nDel = pSprm[2 + mnDelta];
-                sal_uInt8 nIns = pSprm[3 + mnDelta + 4 * nDel];
+                sal_uInt8 nDelIdx = 2 + mnDelta;
+                sal_uInt8 nDel = nDelIdx < nRemLen ? pSprm[nDelIdx] : 0;
+                sal_uInt8 nInsIdx = 3 + mnDelta + 4 * nDel;
+                sal_uInt8 nIns = nInsIdx < nRemLen ? pSprm[nInsIdx] : 0;
 
                 nL = 2 + 4 * nDel + 3 * nIns;
             }
             break;
         case 0xD608:
-            nL = SVBT16ToShort( &pSprm[1 + mnDelta] );
+        {
+            sal_uInt8 nIndex = 1 + mnDelta;
+            if (nIndex + 1 >= nRemLen)
+            {
+                SAL_WARN("sw.ww8", "sprm longer than remaining bytes, doc or parser is wrong");
+                nL = 0;
+            }
+            else
+                nL = SVBT16ToShort(&pSprm[nIndex]);
             break;
+        }
         default:
             switch (aSprm.nVari)
             {
@@ -7939,10 +7950,21 @@ sal_uInt16 wwSprmParser::GetSprmTailLen(sal_uInt16 nId, const sal_uInt8* pSprm)
                     nL = static_cast< sal_uInt16 >(pSprm[1 + mnDelta] + aSprm.nLen);
                     break;
                 case L_VAR2:
+                {
                     // Variable 2-Byte Length?
                     // Excl. Token + Var-Lengthbyte
-                    nL = static_cast< sal_uInt16 >(SVBT16ToShort( &pSprm[1 + mnDelta] ) + aSprm.nLen - 1);
+                    sal_uInt8 nIndex = 1 + mnDelta;
+                    sal_uInt16 nCount;
+                    if (nIndex + 1 >= nRemLen)
+                    {
+                        SAL_WARN("sw.ww8", "sprm longer than remaining bytes, doc or parser is wrong");
+                        nCount = 0;
+                    }
+                    else
+                        nCount = SVBT16ToShort(&pSprm[nIndex]);
+                    nL = static_cast< sal_uInt16 >(nCount + aSprm.nLen - 1);
                     break;
+                }
                 default:
                     OSL_ENSURE(false, "Unknown sprm variant");
                     break;
@@ -7978,9 +8000,9 @@ sal_uInt16 wwSprmParser::GetSprmId(const sal_uInt8* pSp) const
 }
 
 // with tokens and length byte
-sal_uInt16 wwSprmParser::GetSprmSize(sal_uInt16 nId, const sal_uInt8* pSprm) const
+sal_uInt16 wwSprmParser::GetSprmSize(sal_uInt16 nId, const sal_uInt8* pSprm, sal_Int32 nRemLen) const
 {
-    return GetSprmTailLen(nId, pSprm) + 1 + mnDelta + SprmDataOfs(nId);
+    return GetSprmTailLen(nId, pSprm, nRemLen) + 1 + mnDelta + SprmDataOfs(nId);
 }
 
 sal_uInt8 wwSprmParser::SprmDataOfs(sal_uInt16 nId) const
@@ -8000,7 +8022,7 @@ sal_uInt8* wwSprmParser::findSprmData(sal_uInt16 nId, sal_uInt8* pSprms,
     {
         const sal_uInt16 nAktId = GetSprmId(pSprms);
         // set pointer to data
-        sal_uInt16 nSize = GetSprmSize(nAktId, pSprms);
+        sal_uInt16 nSize = GetSprmSize(nAktId, pSprms, nLen);
 
         bool bValid = nSize <= nLen;
 
diff --git a/sw/source/filter/ww8/ww8scan.hxx b/sw/source/filter/ww8/ww8scan.hxx
index 3587a9fabdf5..b71d8885a719 100644
--- a/sw/source/filter/ww8/ww8scan.hxx
+++ b/sw/source/filter/ww8/ww8scan.hxx
@@ -125,7 +125,7 @@ public:
     /// Return the SPRM id at the beginning of this byte sequence
     sal_uInt16 GetSprmId(const sal_uInt8* pSp) const;
 
-    sal_uInt16 GetSprmSize(sal_uInt16 nId, const sal_uInt8* pSprm) const;
+    sal_uInt16 GetSprmSize(sal_uInt16 nId, const sal_uInt8* pSprm, sal_Int32 nRemLen) const;
 
     /// Get known len of a sprms head, the bytes of the sprm id + any bytes
     /// reserved to hold a variable length
@@ -133,7 +133,7 @@ public:
 
     /// Get len of a sprms data area, ignoring the bytes of the sprm id and
     /// ignoring any len bytes. Reports the remaining data after those bytes
-    sal_uInt16 GetSprmTailLen(sal_uInt16 nId, const sal_uInt8 * pSprm) const;
+    sal_uInt16 GetSprmTailLen(sal_uInt16 nId, const sal_uInt8* pSprm, sal_Int32 nRemLen) const;
 
     /// The minimum acceptable sprm len possible for this type of parser
     int MinSprmLen() const { return (IsSevenMinus(meVersion)) ? 2 : 3; }
@@ -262,20 +262,21 @@ private:
     sal_uInt16 nAktId;
     sal_uInt16 nAktSize;
 
-    long nRemLen;   // length of remaining SPRMs (including akt. SPRM)
+    sal_Int32 nRemLen;   // length of remaining SPRMs (including akt. SPRM)
 
     void UpdateMyMembers();
 
 public:
-    explicit WW8SprmIter( const sal_uInt8* pSprms_, long nLen_,
+    explicit WW8SprmIter(const sal_uInt8* pSprms_, sal_Int32 nLen_,
         const wwSprmParser &rSprmParser);
-    void  SetSprms( const sal_uInt8* pSprms_, long nLen_ );
+    void  SetSprms(const sal_uInt8* pSprms_, sal_Int32 nLen_);
     const sal_uInt8* FindSprm(sal_uInt16 nId);
     void  advance();
     const sal_uInt8* GetSprms() const
         { return ( pSprms && (0 < nRemLen) ) ? pSprms : nullptr; }
     const sal_uInt8* GetAktParams() const { return pAktParams; }
     sal_uInt16 GetAktId() const { return nAktId; }
+    sal_Int32 GetRemLen() const { return nRemLen; }
 
 private:
     WW8SprmIter(const WW8SprmIter&) = delete;