vcl/source/filter/graphicfilter2.cxx | 128 ++++++++++++++++++-----------------
1 file changed, 69 insertions(+), 59 deletions(-)
New commits:
commit 4ec93b7c71da27c0e986468e1635412a0c67cc00
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Mon Nov 18 21:19:01 2019 +0300
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Tue Nov 19 14:31:43 2019 +0100
Fix UBSan build after 9fdf8c0a5cc036ea9bd1e11dd8f2c1a6e601fae2
The said commit simplified a testdoc to testTdf128820, using a smallest
possible SVG in it. This seems to produce the smallest possible PNG of
size 8, which is passed into GraphicDescriptor::ImpDetectPNG. There its
size is read into nTemp32 past the end of the file without checks,
which keeps last value of the variable (which was the magic number
0x0d0a1a0a), which is then saved into the descriptor. Then that value
is used in ImpGraphic::ImplGetSizePixel, and later multiplying it in
lclConvertScreenPixelToHmm causes UB.
Fix by checking all the reads in GraphicDescriptor::ImpDetectPNG.
Change-Id: Ib4740fac2b87fbef57d5150151129b9852f3ecb8
Reviewed-on: https://gerrit.libreoffice.org/83119
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
diff --git a/vcl/source/filter/graphicfilter2.cxx
b/vcl/source/filter/graphicfilter2.cxx
index 5640e7c96ed2..0faaaeb81997 100644
--- a/vcl/source/filter/graphicfilter2.cxx
+++ b/vcl/source/filter/graphicfilter2.cxx
@@ -533,83 +533,93 @@ bool GraphicDescriptor::ImpDetectPNG( SvStream& rStm,
bool bExtendedInfo )
if ( bExtendedInfo )
{
- sal_uInt8 cByte = 0;
+ do {
+ sal_uInt8 cByte = 0;
- // IHDR-Chunk
- rStm.SeekRel( 8 );
+ // IHDR-Chunk
+ rStm.SeekRel( 8 );
- // width
- rStm.ReadUInt32( nTemp32 );
- aPixSize.setWidth( nTemp32 );
+ // width
+ rStm.ReadUInt32( nTemp32 );
+ if (!rStm.good())
+ break;
+ aPixSize.setWidth( nTemp32 );
- // height
- rStm.ReadUInt32( nTemp32 );
- aPixSize.setHeight( nTemp32 );
+ // height
+ rStm.ReadUInt32( nTemp32 );
+ if (!rStm.good())
+ break;
+ aPixSize.setHeight( nTemp32 );
- // Bits/Pixel
- rStm.ReadUChar( cByte );
- nBitsPerPixel = cByte;
+ // Bits/Pixel
+ rStm.ReadUChar( cByte );
+ if (!rStm.good())
+ break;
+ nBitsPerPixel = cByte;
- // Colour type - check whether it supports alpha values
- sal_uInt8 cColType = 0;
- rStm.ReadUChar( cColType );
- bIsAlpha = bIsTransparent = ( cColType == 4 || cColType == 6 );
+ // Colour type - check whether it supports alpha values
+ sal_uInt8 cColType = 0;
+ rStm.ReadUChar( cColType );
+ if (!rStm.good())
+ break;
+ bIsAlpha = bIsTransparent = ( cColType == 4 || cColType ==
6 );
- // Planes always 1;
- // compression always
- nPlanes = 1;
+ // Planes always 1;
+ // compression always
+ nPlanes = 1;
- sal_uInt32 nLen32 = 0;
- nTemp32 = 0;
+ sal_uInt32 nLen32 = 0;
+ nTemp32 = 0;
- rStm.SeekRel( 7 );
+ rStm.SeekRel( 7 );
- // read up to the start of the image
- rStm.ReadUInt32( nLen32 );
- rStm.ReadUInt32( nTemp32 );
- while( ( nTemp32 != 0x49444154 ) && rStm.good() )
- {
- if ( nTemp32 == 0x70485973 ) // physical pixel dimensions
+ // read up to the start of the image
+ rStm.ReadUInt32( nLen32 );
+ rStm.ReadUInt32( nTemp32 );
+ while( ( nTemp32 != 0x49444154 ) && rStm.good() )
{
- sal_uLong nXRes;
- sal_uLong nYRes;
+ if ( nTemp32 == 0x70485973 ) // physical pixel
dimensions
+ {
+ sal_uLong nXRes;
+ sal_uLong nYRes;
- // horizontal resolution
- nTemp32 = 0;
- rStm.ReadUInt32( nTemp32 );
- nXRes = nTemp32;
+ // horizontal resolution
+ nTemp32 = 0;
+ rStm.ReadUInt32( nTemp32 );
+ nXRes = nTemp32;
- // vertical resolution
- nTemp32 = 0;
- rStm.ReadUInt32( nTemp32 );
- nYRes = nTemp32;
+ // vertical resolution
+ nTemp32 = 0;
+ rStm.ReadUInt32( nTemp32 );
+ nYRes = nTemp32;
- // unit
- cByte = 0;
- rStm.ReadUChar( cByte );
+ // unit
+ cByte = 0;
+ rStm.ReadUChar( cByte );
- if ( cByte )
- {
- if ( nXRes )
- aLogSize.setWidth( (aPixSize.Width() * 100000)
/ nXRes );
+ if ( cByte )
+ {
+ if ( nXRes )
+ aLogSize.setWidth( (aPixSize.Width() *
100000) / nXRes );
+
+ if ( nYRes )
+ aLogSize.setHeight( (aPixSize.Height() *
100000) / nYRes );
+ }
- if ( nYRes )
- aLogSize.setHeight( (aPixSize.Height() *
100000) / nYRes );
+ nLen32 -= 9;
+ }
+ else if ( nTemp32 == 0x74524e53 ) // transparency
+ {
+ bIsTransparent = true;
+ bIsAlpha = ( cColType != 0 && cColType != 2 );
}
- nLen32 -= 9;
- }
- else if ( nTemp32 == 0x74524e53 ) // transparency
- {
- bIsTransparent = true;
- bIsAlpha = ( cColType != 0 && cColType != 2 );
+ // skip forward to next chunk
+ rStm.SeekRel( 4 + nLen32 );
+ rStm.ReadUInt32( nLen32 );
+ rStm.ReadUInt32( nTemp32 );
}
-
- // skip forward to next chunk
- rStm.SeekRel( 4 + nLen32 );
- rStm.ReadUInt32( nLen32 );
- rStm.ReadUInt32( nTemp32 );
- }
+ } while (false);
}
}
}
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
