From 83488a802d3e6f02faad6accbc17aa5da5795e63 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Thu, 29 Oct 2015 09:51:03 -0700 Subject: XFA: remove unsafe exif parsing code Fortunately, this could only be called with a null buffer, so none of unchecked lengths could be used. The remaining use of the CFX_/IFX_DIBAttributeEx class is as a table, so put one directly in the CFX_DIBAttribute. Fix a "register" warning along the way. R=dsinclair@chromium.org Review URL: https://codereview.chromium.org/1425983002 . --- core/include/fxcodec/fx_codec.h | 18 +- core/src/fxcodec/codec/codec_int.h | 32 --- core/src/fxcodec/codec/fx_codec.cpp | 352 ++----------------------------- core/src/fxcodec/codec/fx_codec_tiff.cpp | 81 ++++--- 4 files changed, 53 insertions(+), 430 deletions(-) (limited to 'core') diff --git a/core/include/fxcodec/fx_codec.h b/core/include/fxcodec/fx_codec.h index 3ac9434861..8eacbb2e96 100644 --- a/core/include/fxcodec/fx_codec.h +++ b/core/include/fxcodec/fx_codec.h @@ -7,6 +7,7 @@ #ifndef CORE_INCLUDE_FXCODEC_FX_CODEC_H_ #define CORE_INCLUDE_FXCODEC_FX_CODEC_H_ +#include #include #include "../../../third_party/base/nonstd_unique_ptr.h" @@ -502,37 +503,24 @@ void AdobeCMYK_to_sRGB1(uint8_t c, uint8_t& G, uint8_t& B); FX_BOOL MD5ComputeID(const void* buf, FX_DWORD dwSize, uint8_t ID[16]); + class CFX_DIBAttribute { public: CFX_DIBAttribute(); ~CFX_DIBAttribute(); int32_t m_nXDPI; - int32_t m_nYDPI; - FX_FLOAT m_fAspectRatio; - FX_WORD m_wDPIUnit; - CFX_ByteString m_strAuthor; - uint8_t m_strTime[20]; - int32_t m_nGifLeft; int32_t m_nGifTop; - FX_DWORD* m_pGifLocalPalette; - FX_DWORD m_nGifLocalPalNum; - int32_t m_nBmpCompressType; - class IFX_DIBAttributeExif* m_pExif; -}; -class IFX_DIBAttributeExif { - public: - virtual ~IFX_DIBAttributeExif(){}; - virtual FX_BOOL GetInfo(FX_WORD tag, void* val) = 0; + std::map m_Exif; }; FX_BOOL FaxSkipEOL(const uint8_t* src_buf, int bitsize, int& bitpos); diff --git a/core/src/fxcodec/codec/codec_int.h b/core/src/fxcodec/codec/codec_int.h index 510d0abe78..36716411b7 100644 --- a/core/src/fxcodec/codec/codec_int.h +++ b/core/src/fxcodec/codec/codec_int.h @@ -385,38 +385,6 @@ class CCodec_Jbig2Module : public ICodec_Jbig2Module { IFX_Pause* pPause) override; void DestroyJbig2Context(void* pJbig2Context) override; }; -class CFX_DIBAttributeExif : public IFX_DIBAttributeExif { - public: - CFX_DIBAttributeExif(); - ~CFX_DIBAttributeExif(); - virtual FX_BOOL GetInfo(FX_WORD tag, void* val); - - FX_BOOL ParseExif(CFX_MapPtrTemplate* pHead, - uint8_t* data, - FX_DWORD len, - CFX_MapPtrTemplate* pVal); - - typedef FX_WORD (*_Read2Bytes)(uint8_t* data); - typedef FX_DWORD (*_Read4Bytes)(uint8_t* data); - uint8_t* ParseExifIFH(uint8_t* data, - FX_DWORD len, - _Read2Bytes* pReadWord, - _Read4Bytes* pReadDword); - FX_BOOL ParseExifIFD(CFX_MapPtrTemplate* pMap, - uint8_t* data, - FX_DWORD len); - - uint8_t* m_pExifData; - - FX_DWORD m_dwExifDataLen; - - void clear(); - _Read2Bytes m_readWord; - _Read4Bytes m_readDword; - CFX_MapPtrTemplate m_TagHead; - CFX_MapPtrTemplate m_TagVal; -}; - struct DecodeData { public: DecodeData(unsigned char* src_data, OPJ_SIZE_T src_size) diff --git a/core/src/fxcodec/codec/fx_codec.cpp b/core/src/fxcodec/codec/fx_codec.cpp index 622dab09d9..a443b75079 100644 --- a/core/src/fxcodec/codec/fx_codec.cpp +++ b/core/src/fxcodec/codec/fx_codec.cpp @@ -257,347 +257,23 @@ FX_BOOL CCodec_BasicModule::A85Encode(const uint8_t* src_buf, FX_DWORD& dest_size) { return FALSE; } -CFX_DIBAttribute::CFX_DIBAttribute() { - FXSYS_memset(this, 0, sizeof(CFX_DIBAttribute)); - m_nXDPI = -1; - m_nYDPI = -1; - m_fAspectRatio = -1.0f; - m_pExif = new CFX_DIBAttributeExif; +CFX_DIBAttribute::CFX_DIBAttribute() + : m_nXDPI(-1), + m_nYDPI(-1), + m_fAspectRatio(-1.0f), + m_wDPIUnit(0), + m_nGifLeft(0), + m_nGifTop(0), + m_pGifLocalPalette(nullptr), + m_nGifLocalPalNum(0), + m_nBmpCompressType(0) { + FXSYS_memset(m_strTime, 0, sizeof(m_strTime)); } CFX_DIBAttribute::~CFX_DIBAttribute() { - if (m_pExif) { - delete m_pExif; - } -} -CFX_DIBAttributeExif::CFX_DIBAttributeExif() { - m_pExifData = NULL; - m_dwExifDataLen = 0; -} -CFX_DIBAttributeExif::~CFX_DIBAttributeExif() { - clear(); -} -void CFX_DIBAttributeExif::clear() { - if (m_pExifData) { - FX_Free(m_pExifData); - } - m_pExifData = NULL; - FX_DWORD key = 0; - uint8_t* buf = NULL; - FX_POSITION pos = NULL; - pos = m_TagHead.GetStartPosition(); - while (pos) { - m_TagHead.GetNextAssoc(pos, key, buf); - if (buf) { - FX_Free(buf); - } - } - m_TagHead.RemoveAll(); - pos = m_TagVal.GetStartPosition(); - while (pos) { - m_TagVal.GetNextAssoc(pos, key, buf); - if (buf) { - FX_Free(buf); - } - } - m_TagVal.RemoveAll(); -} -static FX_WORD _Read2BytesL(uint8_t* data) { - ASSERT(data); - return data[0] | (data[1] << 8); -} -static FX_WORD _Read2BytesB(uint8_t* data) { - ASSERT(data); - return data[1] | (data[0] << 8); -} -static FX_DWORD _Read4BytesL(uint8_t* data) { - return _Read2BytesL(data) | (_Read2BytesL(data + 2) << 16); -} -static FX_DWORD _Read4BytesB(uint8_t* data) { - return _Read2BytesB(data + 2) | (_Read2BytesB(data) << 16); -} -typedef FX_WORD (*_Read2Bytes)(uint8_t* data); -typedef FX_DWORD (*_Read4Bytes)(uint8_t* data); -typedef void (*_Write2Bytes)(uint8_t* data, FX_WORD val); -typedef void (*_Write4Bytes)(uint8_t* data, FX_DWORD val); -uint8_t* CFX_DIBAttributeExif::ParseExifIFH(uint8_t* data, - FX_DWORD len, - _Read2Bytes* pReadWord, - _Read4Bytes* pReadDword) { - if (len > 8) { - FX_BOOL tag = FALSE; - if (FXSYS_memcmp(data, "\x49\x49\x2a\x00", 4) == 0) { - if (pReadWord) { - *pReadWord = _Read2BytesL; - } - if (pReadDword) { - *pReadDword = _Read4BytesL; - } - tag = TRUE; - } else if (FXSYS_memcmp(data, "\x4d\x4d\x00\x2a", 4) == 0) { - if (pReadWord) { - *pReadWord = _Read2BytesB; - } - if (pReadDword) { - *pReadDword = _Read4BytesB; - } - tag = TRUE; - } - if (tag) { - data += 4; - if (pReadDword) { - data += (*pReadDword)(data)-4; - } else { - data += 4; - } - } - } - return data; -} -FX_BOOL CFX_DIBAttributeExif::ParseExifIFD( - CFX_MapPtrTemplate* pMap, - uint8_t* data, - FX_DWORD len) { - if (pMap && data) { - if (len > 8) { - FX_WORD wTagNum = m_readWord(data); - data += 2; - FX_DWORD wTag; - uint8_t* buf; - while (wTagNum--) { - wTag = m_readWord(data); - data += 2; - if (!pMap->Lookup(wTag, buf)) { - buf = FX_Alloc(uint8_t, 10); - if (buf == NULL) { - return FALSE; - } - FXSYS_memcpy(buf, data, 10); - pMap->SetAt(wTag, buf); - } - data += 10; - } - FX_DWORD dwIFDOffset; - dwIFDOffset = m_readDword(data); - while (dwIFDOffset && dwIFDOffset < len) { - data = m_pExifData + dwIFDOffset; - wTagNum = m_readWord(data); - data += 2; - while (wTagNum--) { - wTag = m_readWord(data); - data += 2; - if (!pMap->Lookup(wTag, buf)) { - buf = FX_Alloc(uint8_t, 10); - if (buf == NULL) { - return FALSE; - } - FXSYS_memcpy(buf, data, 10); - pMap->SetAt(wTag, buf); - } - data += 10; - } - dwIFDOffset = m_readDword(data); - } - return TRUE; - } - } - return FALSE; -} -enum FX_ExifDataType { - FX_UnsignedByte = 1, - FX_AscString, - FX_UnsignedShort, - FX_UnsignedLong, - FX_UnsignedRation, - FX_SignedByte, - FX_Undefined, - FX_SignedShort, - FX_SignedLong, - FX_SignedRation, - FX_SignedFloat, - FX_DoubleFloat -}; -FX_BOOL CFX_DIBAttributeExif::ParseExif( - CFX_MapPtrTemplate* pHead, - uint8_t* data, - FX_DWORD len, - CFX_MapPtrTemplate* pVal) { - if (pHead && data && pVal) { - if (len > 8) { - uint8_t* old_data = data; - data = ParseExifIFH(data, len, &m_readWord, &m_readDword); - if (data == old_data) { - return FALSE; - } - if (pHead->GetCount() == 0) { - if (!ParseExifIFD(pHead, data, len)) { - return FALSE; - } - } - FX_DWORD dwModuleNum; - FX_WORD type; - FX_DWORD dwSize; - FX_DWORD tag; - uint8_t* head; - FX_POSITION pos = pHead->GetStartPosition(); - while (pos) { - pHead->GetNextAssoc(pos, tag, head); - uint8_t* val = NULL; - uint8_t* buf = NULL; - uint8_t* temp = NULL; - int i; - if (head) { - type = m_readWord(head); - head += 2; - dwModuleNum = m_readDword(head); - head += 4; - switch (type) { - case FX_UnsignedByte: - case FX_AscString: - case FX_SignedByte: - case FX_Undefined: - dwSize = dwModuleNum; - val = FX_Alloc(uint8_t, dwSize); - if (val == NULL) { - return FALSE; - } - if (dwSize > 4) { - FXSYS_memcpy(val, old_data + m_readDword(head), dwSize); - } else { - FXSYS_memcpy(val, head, dwSize); - } - break; - case FX_UnsignedShort: - case FX_SignedShort: - dwSize = dwModuleNum << 1; - val = FX_Alloc(uint8_t, dwSize); - if (val == NULL) { - return FALSE; - } - if (dwSize > 4) { - FXSYS_memcpy(val, old_data + m_readDword(head), dwSize); - } else { - FXSYS_memcpy(val, head, dwSize); - } - buf = val; - for (i = 0; i < (int)dwModuleNum; i++) { - *(FX_WORD*)buf = m_readWord(buf); - buf += 2; - } - break; - case FX_UnsignedLong: - case FX_SignedLong: - case FX_SignedFloat: - dwSize = dwModuleNum << 2; - val = FX_Alloc(uint8_t, dwSize); - if (val == NULL) { - return FALSE; - } - if (dwSize > 4) { - FXSYS_memcpy(val, old_data + m_readDword(head), dwSize); - } else { - FXSYS_memcpy(val, head, dwSize); - } - buf = val; - for (i = 0; i < (int)dwModuleNum; i++) { - *(FX_DWORD*)buf = m_readDword(buf); - buf += 4; - } - break; - case FX_UnsignedRation: - case FX_SignedRation: { - dwSize = dwModuleNum << 3; - buf = FX_Alloc(uint8_t, dwSize); - if (buf == NULL) { - return FALSE; - } - if (dwSize > 4) { - FXSYS_memcpy(buf, old_data + m_readDword(head), dwSize); - } else { - FXSYS_memcpy(buf, head, dwSize); - } - temp = buf; - val = FX_Alloc(uint8_t, dwSize / 2); - if (val == NULL) { - FX_Free(buf); - return FALSE; - } - for (i = 0; i < (int)dwModuleNum; i++) { - *(FX_DWORD*)temp = m_readDword(temp); - *(FX_DWORD*)(temp + 4) = m_readDword(temp + 4); - FX_DWORD* lNumerator = (FX_DWORD*)temp; - FX_DWORD* lNenominator = (FX_DWORD*)(temp + 4); - *(FX_FLOAT*)(val + i * 4) = - (FX_FLOAT)(*lNumerator) / (FX_FLOAT)(*lNenominator); - temp += 8; - } - FX_Free(buf); - } break; - case FX_DoubleFloat: - dwSize = dwModuleNum << 3; - val = FX_Alloc(uint8_t, dwSize); - if (val == NULL) { - return FALSE; - } - if (dwSize > 4) { - FXSYS_memcpy(val, old_data + m_readDword(head), dwSize); - } else { - FXSYS_memcpy(val, head, dwSize); - } - buf = val; - for (i = 0; i < (int)dwModuleNum; i++) { - *(FX_DWORD*)buf = m_readDword(buf); - *(FX_DWORD*)(buf + 4) = m_readDword(buf + 4); - buf += 8; - } - break; - default: - return FALSE; - } - } - pVal->SetAt(tag, val); - } - return TRUE; - } - } - return FALSE; -} -#define FXEXIF_INFOCONVERT(T) \ - { \ - T* src = (T*)ptr; \ - T* dst = (T*)val; \ - *dst = *src; \ - } -FX_BOOL CFX_DIBAttributeExif::GetInfo(FX_WORD tag, void* val) { - if (m_TagVal.GetCount() == 0) { - if (!ParseExif(&m_TagHead, m_pExifData, m_dwExifDataLen, &m_TagVal)) { - return FALSE; - } - } - uint8_t* ptr = NULL; - if (m_TagVal.Lookup(tag, ptr)) { - switch (tag) { - case EXIFTAG_USHORT_RESUNIT: - FXEXIF_INFOCONVERT(FX_WORD); - { - FX_WORD* ptr = (FX_WORD*)val; - *ptr -= 1; - } - break; - case EXIFTAG_FLOAT_DPIX: - case EXIFTAG_FLOAT_DPIY: - FXEXIF_INFOCONVERT(FX_FLOAT); - break; - case EXIFTAG_USHORT_ORIENTATION: - FXEXIF_INFOCONVERT(FX_WORD); - break; - default: { - uint8_t** dst = (uint8_t**)val; - *dst = ptr; - } - } - } - return TRUE; + for (const auto& pair : m_Exif) + FX_Free(pair.second); } + class CCodec_RLScanlineDecoder : public CCodec_ScanlineDecoder { public: CCodec_RLScanlineDecoder(); diff --git a/core/src/fxcodec/codec/fx_codec_tiff.cpp b/core/src/fxcodec/codec/fx_codec_tiff.cpp index 1f289bbb40..1efd2fbf70 100644 --- a/core/src/fxcodec/codec/fx_codec_tiff.cpp +++ b/core/src/fxcodec/codec/fx_codec_tiff.cpp @@ -256,42 +256,36 @@ void CCodec_TiffContext::GetFrames(int32_t& frames) { } \ } \ (key) = NULL; + +namespace { + template -static FX_BOOL Tiff_Exif_GetInfo(TIFF* tif_ctx, - ttag_t tag, - CFX_DIBAttributeExif* pExif) { - uint8_t* key = NULL; - T val = (T)0; +FX_BOOL Tiff_Exif_GetInfo(TIFF* tif_ctx, ttag_t tag, CFX_DIBAttribute* pAttr) { + T val = 0; TIFFGetField(tif_ctx, tag, &val); - if (val) { - (key) = FX_Alloc(uint8_t, sizeof(T)); - if ((key) == NULL) { - return FALSE; - } - T* ptr = (T*)(key); - *ptr = val; - pExif->m_TagVal.SetAt(tag, (key)); - return TRUE; - } - return FALSE; + if (!val) + return FALSE; + T* ptr = FX_Alloc(T, 1); + *ptr = val; + pAttr->m_Exif[tag] = (void*)ptr; + return TRUE; } -static void Tiff_Exif_GetStringInfo(TIFF* tif_ctx, - ttag_t tag, - CFX_DIBAttributeExif* pExif) { - FX_CHAR* buf = NULL; - uint8_t* key = NULL; +void Tiff_Exif_GetStringInfo(TIFF* tif_ctx, + ttag_t tag, + CFX_DIBAttribute* pAttr) { + FX_CHAR* buf = nullptr; TIFFGetField(tif_ctx, tag, &buf); - if (buf) { - int32_t size = (int32_t)FXSYS_strlen(buf); - (key) = FX_Alloc(uint8_t, size + 1); - if ((key) == NULL) { - return; - } - FXSYS_memcpy((key), buf, size); - key[size] = 0; - pExif->m_TagVal.SetAt(tag, (key)); - } + if (!buf) + return; + FX_STRSIZE size = FXSYS_strlen(buf); + uint8_t* ptr = FX_Alloc(uint8_t, size + 1); + FXSYS_memcpy(ptr, buf, size); + ptr[size] = 0; + pAttr->m_Exif[tag] = ptr; } + +} // namespace + FX_BOOL CCodec_TiffContext::LoadFrameInfo(int32_t frame, FX_DWORD& width, FX_DWORD& height, @@ -322,22 +316,20 @@ FX_BOOL CCodec_TiffContext::LoadFrameInfo(int32_t frame, &pAttribute->m_wDPIUnit)) { pAttribute->m_wDPIUnit -= 1; } - CFX_DIBAttributeExif* pExif = (CFX_DIBAttributeExif*)pAttribute->m_pExif; - pExif->clear(); - Tiff_Exif_GetInfo(tif_ctx, TIFFTAG_ORIENTATION, pExif); - if (Tiff_Exif_GetInfo(tif_ctx, TIFFTAG_XRESOLUTION, pExif)) { - FX_FLOAT fDpi = 0; - pExif->GetInfo(TIFFTAG_XRESOLUTION, &fDpi); + Tiff_Exif_GetInfo(tif_ctx, TIFFTAG_ORIENTATION, pAttribute); + if (Tiff_Exif_GetInfo(tif_ctx, TIFFTAG_XRESOLUTION, pAttribute)) { + void* val = pAttribute->m_Exif[TIFFTAG_XRESOLUTION]; + FX_FLOAT fDpi = val ? *reinterpret_cast(val) : 0; pAttribute->m_nXDPI = (int32_t)(fDpi + 0.5f); } - if (Tiff_Exif_GetInfo(tif_ctx, TIFFTAG_YRESOLUTION, pExif)) { - FX_FLOAT fDpi = 0; - pExif->GetInfo(TIFFTAG_YRESOLUTION, &fDpi); + if (Tiff_Exif_GetInfo(tif_ctx, TIFFTAG_YRESOLUTION, pAttribute)) { + void* val = pAttribute->m_Exif[TIFFTAG_YRESOLUTION]; + FX_FLOAT fDpi = val ? *reinterpret_cast(val) : 0; pAttribute->m_nYDPI = (int32_t)(fDpi + 0.5f); } - Tiff_Exif_GetStringInfo(tif_ctx, TIFFTAG_IMAGEDESCRIPTION, pExif); - Tiff_Exif_GetStringInfo(tif_ctx, TIFFTAG_MAKE, pExif); - Tiff_Exif_GetStringInfo(tif_ctx, TIFFTAG_MODEL, pExif); + Tiff_Exif_GetStringInfo(tif_ctx, TIFFTAG_IMAGEDESCRIPTION, pAttribute); + Tiff_Exif_GetStringInfo(tif_ctx, TIFFTAG_MAKE, pAttribute); + Tiff_Exif_GetStringInfo(tif_ctx, TIFFTAG_MODEL, pAttribute); } bpc = tif_bpc; if (tif_rps > height) { @@ -346,9 +338,8 @@ FX_BOOL CCodec_TiffContext::LoadFrameInfo(int32_t frame, return TRUE; } void _TiffBGRA2RGBA(uint8_t* pBuf, int32_t pixel, int32_t spp) { - register uint8_t tmp; for (int32_t n = 0; n < pixel; n++) { - tmp = pBuf[0]; + uint8_t tmp = pBuf[0]; pBuf[0] = pBuf[2]; pBuf[2] = tmp; pBuf += spp; -- cgit v1.2.3