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

[Caolán McNamara (via logerrit)]

 vcl/source/filter/png/PngImageReader.cxx |   15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

New commits:
commit b34359bcf2f387bc250411a3b78ffcde88329b0e
Author:     Caolán McNamara <caolan.mcnam...@collabora.com>
AuthorDate: Thu Sep 14 17:41:08 2023 +0100
Commit:     Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Thu Sep 14 21:07:47 2023 +0200

    nFrames can't be trusted to believe nSequence exists
    
    Change-Id: Id4745487c97ce89fcf149676c15a974e40ee0eb6
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/156925
    Tested-by: Jenkins
    Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>

diff --git a/vcl/source/filter/png/PngImageReader.cxx 
b/vcl/source/filter/png/PngImageReader.cxx
index f0236490d773..8b6e319a473c 100644
--- a/vcl/source/filter/png/PngImageReader.cxx
+++ b/vcl/source/filter/png/PngImageReader.cxx
@@ -699,9 +699,12 @@ bool reader(SvStream& rStream, Graphic& rGraphic,
         }
         for (sal_uInt32 i = 0; i < nFrames; i++)
         {
-            // Guaranteed to be fcTL chunk here because it was checked earlier
             fcTLChunk* aFctlChunk
-                = 
static_cast<fcTLChunk*>(aAPNGInfo.maFrameData[nSequenceIndex++].get());
+                = nSequenceIndex < aAPNGInfo.maFrameData.size()
+                      ? 
dynamic_cast<fcTLChunk*>(aAPNGInfo.maFrameData[nSequenceIndex++].get())
+                      : nullptr;
+            if (!aFctlChunk)
+                return false;
             Disposal aDisposal = static_cast<Disposal>(aFctlChunk->dispose_op);
             Blend aBlend = static_cast<Blend>(aFctlChunk->blend_op);
             if (i == 0 && aDisposal == Disposal::Back)
@@ -710,7 +713,9 @@ bool reader(SvStream& rStream, Graphic& rGraphic,
             getImportantChunks(rStream, aFrameStream, aFctlChunk->width, 
aFctlChunk->height);
             // A single frame can have multiple fdAT chunks
             while (fdATChunk* pFdatChunk
-                   = 
dynamic_cast<fdATChunk*>(aAPNGInfo.maFrameData[nSequenceIndex].get()))
+                   = nSequenceIndex < aAPNGInfo.maFrameData.size()
+                         ? 
dynamic_cast<fdATChunk*>(aAPNGInfo.maFrameData[nSequenceIndex].get())
+                         : nullptr)
             {
                 // Write fdAT chunks as IDAT chunks
                 auto nDataSize = pFdatChunk->frame_data.size();
@@ -719,8 +724,6 @@ bool reader(SvStream& rStream, Graphic& rGraphic,
                 sal_uInt32 nCrc = rtl_crc32(0, pFdatChunk->frame_data.data(), 
nDataSize);
                 aFrameStream.WriteUInt32(nCrc);
                 nSequenceIndex++;
-                if (nSequenceIndex >= aAPNGInfo.maFrameData.size())
-                    break;
             }
             aFrameStream.WriteUInt32(PNG_IEND_SIZE);
             aFrameStream.WriteUInt32(PNG_IEND_SIGNATURE);
commit 0aa3e40a733230ae439c98244f6db2f97ede1b01
Author:     Caolán McNamara <caolan.mcnam...@collabora.com>
AuthorDate: Thu Sep 14 17:29:33 2023 +0100
Commit:     Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Thu Sep 14 21:07:39 2023 +0200

    ofz#62352 Null-dereference READ
    
    Change-Id: Ibf8aa63a6d63809b361902b853dabd8ff0944503
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/156924
    Tested-by: Jenkins
    Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>

diff --git a/vcl/source/filter/png/PngImageReader.cxx 
b/vcl/source/filter/png/PngImageReader.cxx
index fcdcd210a157..f0236490d773 100644
--- a/vcl/source/filter/png/PngImageReader.cxx
+++ b/vcl/source/filter/png/PngImageReader.cxx
@@ -676,6 +676,8 @@ bool reader(SvStream& rStream, Graphic& rGraphic,
         sal_uInt32 nFrames
             = aAPNGInfo.maACTLChunk.num_frames - 
static_cast<sal_uInt32>(bFctlBeforeIDAT);
         {
+            if (aAPNGInfo.maFrameData.empty())
+                return false;
             fcTLChunk* aFctlChunk = 
dynamic_cast<fcTLChunk*>(aAPNGInfo.maFrameData[0].get());
             if (!aFctlChunk)
                 return false;