[prev in list] [next in list] [prev in thread] [next in thread]
List: helix-datatype-cvs
Subject: [Datatype-cvs] image/gif/common gifcodec.cpp, 1.3.6.3,
From: bobclark () helixcommunity ! org
Date: 2008-08-26 21:57:38
Message-ID: 200808262201.m7QM1tLg005380 () mailer ! progressive-comp ! com
[Download RAW message or body]
Update of /cvsroot/datatype/image/gif/common
In directory cvs01.internal.helixcommunity.org:/tmp/cvs-serv12986
Modified Files:
Tag: hxclient_1_5_0_cayenne
gifcodec.cpp gifimage.cpp
Log Message:
merge fix for 219759 into 150Cay.
Index: gifimage.cpp
===================================================================
RCS file: /cvsroot/datatype/image/gif/common/gifimage.cpp,v
retrieving revision 1.3
retrieving revision 1.3.6.1
diff -u -d -r1.3 -r1.3.6.1
--- gifimage.cpp 9 Jul 2004 18:30:22 -0000 1.3
+++ gifimage.cpp 26 Aug 2008 21:57:27 -0000 1.3.6.1
@@ -203,12 +203,21 @@
* If we're at an
*/
BYTE *pBufPtr = pBuffer;
+ BYTE *pBufLimit = pBufPtr + ulLen;
+ if (pBufPtr + 1 > pBufLimit)
+ {
+ return HXR_UNEXPECTED;
+ }
if (*pBufPtr == CGIFCodec::kExtension)
{
/* Advance the pointer to the extension type */
pBufPtr++;
/* Verify the extension is a GCE */
+ if (pBufPtr + 1 > pBufLimit)
+ {
+ return HXR_UNEXPECTED;
+ }
UINT32 ulExtension = *pBufPtr++;
if (ulExtension != CGIFCodec::kGraphicControlExtension)
{
@@ -221,12 +230,20 @@
UINT32 ulBlockSize;
do
{
+ if (pBufPtr + 1 > pBufLimit)
+ {
+ return HXR_UNEXPECTED;
+ }
/* Get the block size */
ulBlockSize = *pBufPtr++;
/* Initialize the GCE */
if (ulBlockSize >= 4)
{
+ if (pBufPtr + 4 > pBufLimit)
+ {
+ return HXR_UNEXPECTED;
+ }
ParseGraphicControlExtension(pBufPtr, m_cGCE);
}
@@ -241,21 +258,24 @@
// {
// return HXR_UNEXPECTED;
// }
- while (*pBufPtr != CGIFCodec::kImageDescriptor && pBufPtr < pBuffer + ulLen)
+ while (pBufPtr < pBufLimit && *pBufPtr != CGIFCodec::kImageDescriptor)
{
// We assume here that these can only be extensions
pBufPtr += 2;
- CGIFCodec::SkipBlocks(pBufPtr);
- }
- if (pBufPtr >= pBuffer + ulLen)
- {
- return HXR_FAIL;
+ if (CGIFCodec::SkipBlocks(pBufPtr, pBufLimit) != HXR_OK)
+ {
+ return HXR_FAIL;
+ }
}
/* Advance the pointer */
pBufPtr++;
/* Parse the Image Descriptor */
+ if (pBufPtr + 9 > pBufLimit)
+ {
+ return HXR_UNEXPECTED;
+ }
ParseImageDescriptor(pBufPtr, m_cID);
pBufPtr += 9;
@@ -276,6 +296,10 @@
}
/* Read in the local color table */
+ if (pBufPtr + ulNumBytes > pBufLimit)
+ {
+ return HXR_UNEXPECTED;
+ }
memcpy(m_pucLocalColorMap, pBufPtr, ulNumBytes); /* Flawfinder: ignore */
/* Advance the pointer */
@@ -342,6 +366,8 @@
return HXR_UNEXPECTED;
}
+ BYTE* pBufLimit = pBuffer + ulLen;
+
/* If the state is kStateDecoInitialized, then
* this is the first time in Decompress(). Therefore,
* we need to initialize the LZWCodec.
@@ -353,6 +379,10 @@
* To initialize, we have to extract the first byte of the buffer,
* which is the minimum LZW code size.
*/
+ if (pBuffer + 1 > pBufLimit)
+ {
+ return HXR_UNEXPECTED;
+ }
INT32 lSize = *pBuffer++;
/* Decrement the length */
@@ -383,6 +413,10 @@
do
{
/* Read the block size */
+ if (pBuffer + 1 > pBufLimit)
+ {
+ return HXR_UNEXPECTED;
+ }
ulLZWBlockSize = *pBuffer++;
/* Decrement the length */
Index: gifcodec.cpp
===================================================================
RCS file: /cvsroot/datatype/image/gif/common/gifcodec.cpp,v
retrieving revision 1.3.6.3
retrieving revision 1.3.6.4
diff -u -d -r1.3.6.3 -r1.3.6.4
--- gifcodec.cpp 17 Mar 2005 17:30:58 -0000 1.3.6.3
+++ gifcodec.cpp 26 Aug 2008 21:57:27 -0000 1.3.6.4
@@ -170,6 +170,11 @@
if (pBuf && ulLen)
{
BYTE* pBufLimit = pBuf + ulLen;
+ // Make sure there's at least a header and logical screen descriptor.
+ if (pBuf + 13 > pBufLimit)
+ {
+ return ulNumPackets;
+ }
if (pBuf[0] == 'G' && pBuf[1] == 'I' && pBuf[2] == 'F' &&
pBuf[3] == '8' && pBuf[5] == 'a')
{
@@ -197,6 +202,11 @@
// Skip the Image Descriptor marker
pBuf++;
// Parse the image descriptor struct
+ // First verify available buffer.
+ if (pBuf + 9 > pBufLimit)
+ {
+ return ulNumPackets;
+ }
CGIFImage::ImageDescriptor cID;
CGIFImage::ParseImageDescriptor(pBuf, cID);
pBuf += 9;
@@ -213,6 +223,11 @@
UINT32 ulBlockSize = 0;
do
{
+ // Check for end of buffer before reading.
+ if (pBuf + 1 > pBufLimit)
+ {
+ return ulNumPackets;
+ }
// Get the block size
ulBlockSize = *pBuf++;
// Skip that amount of bytes
@@ -230,6 +245,11 @@
UINT32 ulBlockSize = 0;
do
{
+ // Check for end of buffer before reading.
+ if (pBuf + 1 > pBufLimit)
+ {
+ return ulNumPackets;
+ }
// Get the block size
ulBlockSize = *pBuf++;
// Skip that amount of bytes
@@ -298,6 +318,12 @@
BYTE *pBuf = m_pParseBuffer;
BYTE *pBufLimit = pBuf + m_ulParseBufferLength;
+ // Check for sufficient buffer for header and logical screen descriptor.
+ if (pBuf + 13 > pBufLimit)
+ {
+ return HXR_INVALID_OPERATION;
+ }
+
/* Check the signature */
if (pBuf[0] != 'G' || pBuf[1] != 'I' || pBuf[2] != 'F')
{
@@ -369,6 +395,11 @@
* Whether we mark it or not, we still have to parse it to find out
* whether or not a local color table is present.
*/
+ // Make sure there's enough buffer to contain in image descriptor before \
parsing. + if (pBuf + 9 > pBufLimit)
+ {
+ return HXR_FAIL;
+ }
CGIFImage::ImageDescriptor cID;
CGIFImage::ParseImageDescriptor(pBuf, cID);
pBuf += 9;
@@ -394,6 +425,11 @@
cMarkList.PushBack((void *) pBuf);
/* Skip the minimum LZW code size */
+ // Check for buffer available before reading InitialCodeSize.
+ if (pBuf + 1 > pBufLimit)
+ {
+ return HXR_FAIL;
+ }
UINT32 ulInitialCodeSize = *pBuf++;
if (ulInitialCodeSize > 12)
{
@@ -404,7 +440,10 @@
}
/* Now skip the LZW blocks */
- SkipBlocks(pBuf, pBufLimit);
+ if (SkipBlocks(pBuf, pBufLimit) != HXR_OK)
+ {
+ return HXR_FAIL;
+ }
// Check to see if we went past the end of the buffer
if (pBuf >= pBufLimit)
@@ -423,6 +462,11 @@
else if (pBuf[0] == kExtension)
{
/* This is an extension, but what type? */
+ // If no extension type in the buffer, fail.
+ if (pBuf + 2 > pBufLimit)
+ {
+ return HXR_FAIL;
+ }
switch(pBuf[1])
{
case kGraphicControlExtension:
@@ -434,10 +478,19 @@
do
{
/* Get the block size */
+ // Check for available buffer first.
+ if (pBuf + 1 > pBufLimit)
+ {
+ return HXR_FAIL;
+ }
ulBlockSize = *pBuf++;
/* Initialize the GCE */
CGIFImage::GraphicControlExtension cGCE;
+ if (pBuf + ulBlockSize > pBufLimit)
+ {
+ return HXR_FAIL;
+ }
if (ulBlockSize >= 4)
{
CGIFImage::ParseGraphicControlExtension(pBuf, cGCE);
@@ -454,16 +507,25 @@
case kCommentExtension:
case kPlainTextExtension:
pBuf += 2;
- SkipBlocks(pBuf, pBufLimit);
+ if (SkipBlocks(pBuf, pBufLimit) != HXR_OK)
+ {
+ return HXR_FAIL;
+ }
break;
case kApplicationExtension:
- ParseApplicationExtension(pBuf);
+ if (ParseApplicationExtension(pBuf, pBufLimit) != HXR_OK)
+ {
+ return HXR_FAIL;
+ }
break;
default:
// An extension we don't know - just try to
// pass it through
pBuf += 2;
- SkipBlocks(pBuf, pBufLimit);
+ if (SkipBlocks(pBuf, pBufLimit) != HXR_OK)
+ {
+ return HXR_FAIL;
+ }
break;
}
@@ -614,6 +676,11 @@
/* Is this our first time here? */
if (m_ulParseState == kStateParseInitialized)
{
+ // Make sure we haven't been fed a totally bogus NumImages somewhere along \
the way. + if (m_ulNumImages > (INT_MAX - m_pSegment[0].ulSize - 8)/8)
+ {
+ return HXR_UNEXPECTED;
+ }
/* This is the first time here - we need to find the header length */
UINT32 ulLen = 8 + /* Container header size, num images \
*/
8 * m_ulNumImages + /* Image header size and compressed \
data size */ @@ -809,9 +876,13 @@
cLSD.m_fPixelAspectRatio = (cLSD.m_ulPixelAspectRatio + 15.0F) / 64.0F;
}
-HX_RESULT CGIFCodec::ParseContainerHeader(BYTE * &pBuffer)
+HX_RESULT CGIFCodec::ParseContainerHeader(BYTE * &pBuffer, BYTE* const pBufLimit)
{
/* Verify the signature */
+ if (pBuffer + 13 > pBufLimit)
+ {
+ return HXR_UNEXPECTED;
+ }
if (pBuffer[0] != 'G' || pBuffer[1] != 'I' || pBuffer[2] != 'F')
{
return HXR_INVALID_OPERATION;
@@ -854,6 +925,10 @@
}
/* Copy the global color table */
+ if (pBuffer + ulColorTableBytes > pBufLimit)
+ {
+ return HXR_UNEXPECTED;
+ }
memcpy(m_pucGlobalColorMap, pBuffer, ulColorTableBytes); /* Flawfinder: \
ignore */
/* Advance the pointer */
@@ -894,6 +969,10 @@
ulImageNum++;
break;
case kExtension:
+ if (pBuffer + 2 > pBufLimit)
+ {
+ return HXR_UNEXPECTED;
+ }
if (pBuffer[1] == kGraphicControlExtension)
{
retVal = m_pImage[ulImageNum].InitDecompress(pBuffer, \
m_pImageHeaderSize[ulImageNum]); @@ -913,14 +992,20 @@
}
else if (pBuffer[1] == kApplicationExtension)
{
- ParseApplicationExtension(pBuffer);
+ if(ParseApplicationExtension(pBuffer, pBufLimit) != HXR_OK)
+ {
+ return HXR_FAIL;
+ }
}
else
{
/* Skip the extension marker and type */
pBuffer += 2;
/* Now skip the extension itself */
- SkipBlocks(pBuffer);
+ if (SkipBlocks(pBuffer, pBufLimit) != HXR_OK)
+ {
+ return HXR_FAIL;
+ }
}
break;
case kTrailer:
@@ -940,22 +1025,36 @@
return HXR_OK;
}
-void CGIFCodec::SkipBlocks(BYTE * &pBuffer, BYTE* pBufLimit)
+HX_RESULT CGIFCodec::SkipBlocks(BYTE * &pBuffer, BYTE* pBufLimit)
{
- // If pBufLimit is NULL, then we won't use it at all.
- // If pBufLimit is not NULL, then we will make sure
- // we don't go past the end of the buffer
- UINT32 ulBlockSize;
- do
+ HX_RESULT res = HXR_OK;
+
+ UINT32 ulBlockSize = 0;
+
+ if (!pBuffer || !pBufLimit || pBuffer >= pBufLimit)
{
- /* Get the block size */
- ulBlockSize = *pBuffer++;
+ res = HXR_UNEXPECTED;
+ }
+ else
+ {
+ do
+ {
+ /* Get the block size */
+ ulBlockSize = *pBuffer++;
- /* Skip that amount of bytes */
- pBuffer += ulBlockSize;
+ /* Skip that amount of bytes */
+ pBuffer += ulBlockSize;
+ }
+ while (ulBlockSize > 0 && pBuffer < pBufLimit);
+
+ // If we're at the end, then this file is corrupted.
+ if (pBuffer >= pBufLimit)
+ {
+ res = HXR_UNEXPECTED;
+ }
}
- while (ulBlockSize > 0 &&
- (!pBufLimit || (pBufLimit && pBuffer < pBufLimit)));
+
+ return res;
}
HX_RESULT CGIFCodec::InitDecompress(BYTE *pBuffer, UINT32 ulLen)
@@ -966,20 +1065,32 @@
return HXR_INVALID_PARAMETER;
}
+ BYTE* pBufLimit = pBuffer + ulLen;
+
/* Check the state */
if (m_ulState != kStateConstructed)
{
return HXR_UNEXPECTED;
}
- /* Get the master header length */
- UnPack32(pBuffer);
+ /* Skip the master header length */
pBuffer += 4;
/* Get the number of images */
+ if (pBuffer + 4 > pBufLimit)
+ {
+ return HXR_UNEXPECTED;
+ }
m_ulNumImages = UnPack32(pBuffer);
pBuffer += 4;
- if (m_ulNumImages == 0)
+ // Since the buffer must hold 8 bytes per image for the
+ // ImageHeaderSize and CompressedBufferSize, (ulLen / 8) is a
+ // reasonable limit to the number of images that can be in the
+ // buffer. This limit should never be reached, because the buffer
+ // should also contain the actual image data and miscellaneous info.
+ // This check will allows us to fail before allocating tons of memory
+ // if we get fed a bogus large number.
+ if (m_ulNumImages == 0 || m_ulNumImages > (ulLen / 8))
{
return HXR_UNEXPECTED;
}
@@ -1040,6 +1151,20 @@
UINT32 i;
for (i = 0; i < m_ulNumImages; i++)
{
+ if (pBuffer + 8 > pBufLimit)
+ {
+ if (m_pImage)
+ {
+ delete [] m_pImage;
+ m_pImage = NULL;
+ }
+ if (m_pImageHeaderSize)
+ {
+ delete [] m_pImageHeaderSize;
+ m_pImageHeaderSize = NULL;
+ }
+ return HXR_UNEXPECTED;
+ }
/* Get the image header size */
m_pImageHeaderSize[i] = UnPack32(pBuffer);
pBuffer += 4;
@@ -1050,7 +1175,7 @@
}
/* Now parse the container header */
- retVal = ParseContainerHeader(pBuffer);
+ retVal = ParseContainerHeader(pBuffer, pBufLimit);
if (retVal != HXR_OK)
{
if (m_pImage)
@@ -1168,7 +1293,9 @@
UINT32 ulPadWidth, BOOL bRowsInverted)
{
// Check for input error
- if (lCurIndex < -1 || lCurIndex >= (INT32) m_ulNumImages || ulImgIndex >= \
m_ulNumImages || + if (lCurIndex < -1 ||
+ m_ulNumImages > INT_MAX || // Check NumImages is in proper range before \
casting. + lCurIndex >= (INT32) m_ulNumImages || ulImgIndex >= m_ulNumImages \
||
pBuffer == NULL || ulWidth == 0 || ulHeight == 0 || ulPadWidth == 0)
{
return HXR_INVALID_PARAMETER;
@@ -1198,7 +1325,9 @@
BYTE ucBackAlpha)
{
// Check for input error
- if (lCurIndex < -1 || lCurIndex >= (INT32) m_ulNumImages || ulImgIndex >= \
m_ulNumImages || + if (lCurIndex < -1 ||
+ m_ulNumImages > INT_MAX || // Check NumImages is in range before casting.
+ lCurIndex >= (INT32) m_ulNumImages || ulImgIndex >= m_ulNumImages ||
pBuffer == NULL || ulWidth == 0 || ulHeight == 0 || ulPadWidth == 0 || \
ulBytesPerPixel == 0) {
return HXR_INVALID_PARAMETER;
@@ -1231,7 +1360,9 @@
{
HX_RESULT retVal = HXR_OK;
- if (lCurIndex >= -1 && lCurIndex < (INT32) m_ulNumImages &&
+ if (lCurIndex >= -1 &&
+ m_ulNumImages <= INT_MAX && m_ulNumImages >= 0 && // Check numImages is in \
proper range before casting. + lCurIndex < (INT32) m_ulNumImages &&
ulImgIndex < m_ulNumImages && pBuffer && ulWidth && ulHeight &&
ulPadWidth && ulBytesPerPixel)
{
@@ -1304,45 +1435,70 @@
return FALSE;
}
-void CGIFCodec::ParseApplicationExtension(BYTE * &pBuf)
+HX_RESULT CGIFCodec::ParseApplicationExtension(BYTE * &pBuf, BYTE* const pBufLimit)
{
+ HX_RESULT res = HXR_OK;
+
pBuf += 2; // skip the extension introducer and the application extension label
// The next block should always be 11 bytes - 8 bytes for application identifier
// and 3 bytes for the application authentication code. If it's not then just
// skip these blocks
- if (pBuf[0] == 11)
+ if (pBuf + 1 > pBufLimit)
{
- // Now we check to see if this is a NETSCAPE2.0 application extension.
- // If it is, then it contains the loop count for the animation.
- if (!strncmp((const char *) pBuf + 1, "NETSCAPE2.0", 11))
+ res = HXR_UNEXPECTED;
+ }
+ else
+ {
+ if (pBuf[0] == 11)
{
- // Yep, we've got a NETSCAPE2.0 application extension,
- // so attempt to extract the loop count
- if (pBuf[12] == 0x03 && pBuf[13] == 0x01 && pBuf[16] == 0x00)
+ // Now we check to see if this is a NETSCAPE2.0 application extension.
+ // If it is, then it contains the loop count for the animation.
+ if (pBuf + 12 > pBufLimit)
{
- UINT32 ulCount = (pBuf[15] << 8) | pBuf[14];
- if (ulCount == 0)
+ res = HXR_UNEXPECTED;
+ }
+ else
+ {
+ if (!strncmp((const char *) pBuf + 1, "NETSCAPE2.0", 11))
{
- m_ulLoopCount = 0;
+ // Yep, we've got a NETSCAPE2.0 application extension,
+ // so attempt to extract the loop count
+ if (pBuf + 17 > pBufLimit)
+ {
+ res = HXR_UNEXPECTED;
+ }
+ else
+ {
+ if (pBuf[12] == 0x03 && pBuf[13] == 0x01 && pBuf[16] == \
0x00) + {
+ UINT32 ulCount = (pBuf[15] << 8) | pBuf[14];
+ if (ulCount == 0)
+ {
+ m_ulLoopCount = 0;
+ }
+ else
+ {
+ m_ulLoopCount = ulCount + 1;
+ }
+ pBuf += 17;
+ }
+ else
+ {
+ res = SkipBlocks(pBuf, pBufLimit);
+ }
+ }
}
else
{
- m_ulLoopCount = ulCount + 1;
+ res = SkipBlocks(pBuf, pBufLimit);
}
- pBuf += 17;
- }
- else
- {
- SkipBlocks(pBuf);
}
}
else
{
- SkipBlocks(pBuf);
+ res = SkipBlocks(pBuf, pBufLimit);
}
}
- else
- {
- SkipBlocks(pBuf);
- }
+
+ return res;
}
_______________________________________________
Datatype-cvs mailing list
Datatype-cvs@helixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/datatype-cvs
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic