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 .

4 files changed, 53 insertions, 430 deletions

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 <map>
 #include <vector>
 
 #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<FX_DWORD, void*> 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<FX_DWORD, uint8_t*>* pHead,
-                    uint8_t* data,
-                    FX_DWORD len,
-                    CFX_MapPtrTemplate<FX_DWORD, uint8_t*>* 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<FX_DWORD, uint8_t*>* 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<FX_DWORD, uint8_t*> m_TagHead;
-  CFX_MapPtrTemplate<FX_DWORD, uint8_t*> 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<FX_DWORD, uint8_t*>* 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<FX_DWORD, uint8_t*>* pHead,
-    uint8_t* data,
-    FX_DWORD len,
-    CFX_MapPtrTemplate<FX_DWORD, uint8_t*>* 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 <class T>

-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<FX_WORD>(tif_ctx, TIFFTAG_ORIENTATION, pExif);

-    if (Tiff_Exif_GetInfo<FX_FLOAT>(tif_ctx, TIFFTAG_XRESOLUTION, pExif)) {

-      FX_FLOAT fDpi = 0;

-      pExif->GetInfo(TIFFTAG_XRESOLUTION, &fDpi);

+    Tiff_Exif_GetInfo<FX_WORD>(tif_ctx, TIFFTAG_ORIENTATION, pAttribute);

+    if (Tiff_Exif_GetInfo<FX_FLOAT>(tif_ctx, TIFFTAG_XRESOLUTION, pAttribute)) {

+      void* val = pAttribute->m_Exif[TIFFTAG_XRESOLUTION];

+      FX_FLOAT fDpi = val ? *reinterpret_cast<FX_FLOAT*>(val) : 0;

       pAttribute->m_nXDPI = (int32_t)(fDpi + 0.5f);

     }

-    if (Tiff_Exif_GetInfo<FX_FLOAT>(tif_ctx, TIFFTAG_YRESOLUTION, pExif)) {

-      FX_FLOAT fDpi = 0;

-      pExif->GetInfo(TIFFTAG_YRESOLUTION, &fDpi);

+    if (Tiff_Exif_GetInfo<FX_FLOAT>(tif_ctx, TIFFTAG_YRESOLUTION, pAttribute)) {

+      void* val = pAttribute->m_Exif[TIFFTAG_YRESOLUTION];

+      FX_FLOAT fDpi = val ? *reinterpret_cast<FX_FLOAT*>(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;