Merge pull request OSGeo#13598 from rouault/jpeg_and_exif_large_offsets

JPEG/EXIF: make them handle large offsets

Author: rouault

frmts/avif/avifdataset.cpp

@@ -592,16 +592,16 @@ bool GDALAVIFDataset::Init(GDALOpenInfo *poOpenInfo)
         VSILFILE *fpEXIF =
             VSIFileFromMemBuffer(nullptr, m_decoder->image->exif.data,
                                  m_decoder->image->exif.size, false);
-        int nExifOffset = 0;
-        int nInterOffset = 0;
-        int nGPSOffset = 0;
+        uint32_t nExifOffset = 0;
+        uint32_t nInterOffset = 0;
+        uint32_t nGPSOffset = 0;
         char **papszEXIFMetadata = nullptr;
 #ifdef CPL_LSB
         const bool bSwab = m_decoder->image->exif.data[0] == 0x4d;
 #else
         const bool bSwab = m_decoder->image->exif.data[0] == 0x49;
 #endif
-        constexpr int nTIFFHEADER = 0;
+        constexpr uint32_t nTIFFHEADER = 0;
         uint32_t nTiffDirStart;
         memcpy(&nTiffDirStart, m_decoder->image->exif.data + 4,
                sizeof(uint32_t));

frmts/gtiff/gtiffdataset_read.cpp

@@ -6599,24 +6599,24 @@ void GTiffDataset::LoadEXIFMetadata()
     const bool bSwabflag = bLittleEndian != bLeastSignificantBit;  // != is XOR.
 
     char **papszMetadata = nullptr;
-    toff_t nOffset = 0;  // TODO(b/28199387): Refactor to simplify casting.
+    toff_t nOffset = 0;
 
     if (TIFFGetField(m_hTIFF, TIFFTAG_EXIFIFD, &nOffset))
     {
-        int nExifOffset = static_cast<int>(nOffset);
-        int nInterOffset = 0;
-        int nGPSOffset = 0;
-        EXIFExtractMetadata(papszMetadata, fp, static_cast<int>(nOffset),
+        uint32_t nExifOffset = static_cast<uint32_t>(nOffset);
+        uint32_t nInterOffset = 0;
+        uint32_t nGPSOffset = 0;
+        EXIFExtractMetadata(papszMetadata, fp, static_cast<uint32_t>(nOffset),
                             bSwabflag, 0, nExifOffset, nInterOffset,
                             nGPSOffset);
     }
 
     if (TIFFGetField(m_hTIFF, TIFFTAG_GPSIFD, &nOffset))
     {
-        int nExifOffset = 0;  // TODO(b/28199387): Refactor to simplify casting.
-        int nInterOffset = 0;
-        int nGPSOffset = static_cast<int>(nOffset);
-        EXIFExtractMetadata(papszMetadata, fp, static_cast<int>(nOffset),
+        uint32_t nExifOffset = 0;
+        uint32_t nInterOffset = 0;
+        uint32_t nGPSOffset = static_cast<uint32_t>(nOffset);
+        EXIFExtractMetadata(papszMetadata, fp, static_cast<uint32_t>(nOffset),
                             bSwabflag, 0, nExifOffset, nInterOffset,
                             nGPSOffset);
     }

frmts/heif/heifdataset.cpp

@@ -460,15 +460,15 @@ void GDALHEIFDataset::ReadMetadata()
             constexpr bool bLSBPlatform = CPL_IS_LSB != 0;
             const bool bSwabflag = bLittleEndianTIFF != bLSBPlatform;
 
-            int nTIFFDirOff;
+            uint32_t nTIFFDirOff;
             memcpy(&nTIFFDirOff, &data[nTIFFFileOffset + 4], 4);
             if (bSwabflag)
             {
                 CPL_SWAP32PTR(&nTIFFDirOff);
             }
-            int nExifOffset = 0;
-            int nInterOffset = 0;
-            int nGPSOffset = 0;
+            uint32_t nExifOffset = 0;
+            uint32_t nInterOffset = 0;
+            uint32_t nGPSOffset = 0;
             EXIFExtractMetadata(papszMD, fpTemp, nTIFFDirOff, bSwabflag, 0,
                                 nExifOffset, nInterOffset, nGPSOffset);
             if (nExifOffset > 0)

frmts/jpeg/jpgdataset.cpp

@@ -18,6 +18,7 @@
 #include "jpgdataset.h"
 
 #include <cerrno>
+#include <cinttypes>
 #include <climits>
 #include <cstddef>
 #include <cstdio>
@@ -149,7 +150,7 @@ void JPGDatasetCommon::ReadImageStructureMetadata()
     const vsi_l_offset nCurOffset = VSIFTellL(m_fpImage);
 
     GByte abyChunkHeader[4];
-    int nChunkLoc = 2;
+    vsi_l_offset nChunkLoc = 2;
     constexpr GByte MARKER_QUANT_TABLE = 0xDB;
     struct CPLMD5Context context;
     CPLMD5Init(&context);
@@ -331,7 +332,7 @@ void JPGDatasetCommon::ReadXMPMetadata()
     constexpr int APP1_XMP_SIGNATURE_LEN =
         static_cast<int>(sizeof(APP1_XMP_SIGNATURE));
     GByte abyChunkHeader[JFIF_MARKER_SIZE + APP1_XMP_SIGNATURE_LEN] = {};
-    int nChunkLoc = 2;
+    vsi_l_offset nChunkLoc = 2;
     bool bFoundXMP = false;
 
     while (true)
@@ -433,7 +434,7 @@ void JPGDatasetCommon::ReadDJIMetadata()
 
     const vsi_l_offset nCurOffset = VSIFTellL(m_fpImage);
 
-    int nChunkLoc = 2;
+    vsi_l_offset nChunkLoc = 2;
     // size of APP1 segment marker"
     GByte abyChunkHeader[4];
 
@@ -552,7 +553,7 @@ void JPGDatasetCommon::ReadFLIRMetadata()
     // Save current position to avoid disturbing JPEG stream decoding.
     const vsi_l_offset nCurOffset = VSIFTellL(m_fpImage);
 
-    int nChunkLoc = 2;
+    vsi_l_offset nChunkLoc = 2;
     // size of APP1 segment marker + size of "FLIR\0"
     GByte abyChunkHeader[4 + 5];
     std::vector<GByte> abyFLIR;
@@ -1198,7 +1199,7 @@ void JPGDatasetCommon::ReadICCProfile()
 
     // Search for APP2 chunk.
     GByte abyChunkHeader[18] = {};
-    int nChunkLoc = 2;
+    vsi_l_offset nChunkLoc = 2;
     bool bOk = true;
 
     while (true)
@@ -1378,10 +1379,9 @@ void JPGDatasetCommon::ReadICCProfile()
 /************************************************************************/
 bool JPGDatasetCommon::EXIFInit(VSILFILE *fp)
 {
-    if (nTiffDirStart == 0)
-        return false;
-    if (nTiffDirStart > 0)
-        return true;
+    if (m_bTiffDirStartInit)
+        return nTiffDirStart > 0;
+    m_bTiffDirStartInit = true;
     nTiffDirStart = 0;
 
 #ifdef CPL_MSB
@@ -1392,7 +1392,7 @@ bool JPGDatasetCommon::EXIFInit(VSILFILE *fp)
 
     // Search for APP1 chunk.
     GByte abyChunkHeader[10] = {};
-    int nChunkLoc = 2;
+    vsi_l_offset nChunkLoc = 2;
 
     while (true)
     {
@@ -1431,25 +1431,27 @@ bool JPGDatasetCommon::EXIFInit(VSILFILE *fp)
                 STARTS_WITH(reinterpret_cast<char *>(abyChunkHeader) + 4,
                             "Exif"))
             {
-                if (nTIFFHEADER < 0)
+                if (nTIFFHEADER == 0)
                 {
                     nTIFFHEADER = nChunkLoc + 10;
                 }
                 else
                 {
-                    CPLDebug(
-                        "JPEG",
-                        "Another Exif directory found at offset %u. Ignoring "
-                        "it and only taking into account the one at offset %u",
-                        unsigned(nChunkLoc + 10), unsigned(nTIFFHEADER));
+                    CPLDebug("JPEG",
+                             "Another Exif directory found at offset %" PRIu64
+                             ". Ignoring "
+                             "it and only taking into account the one at "
+                             "offset %" PRIu64,
+                             static_cast<uint64_t>(nChunkLoc + 10),
+                             static_cast<uint64_t>(nTIFFHEADER));
                 }
             }
         }
 
         nChunkLoc += 2 + nChunkLength;
     }
 
-    if (nTIFFHEADER < 0)
+    if (nTIFFHEADER == 0)
         return false;
 
     // Read TIFF header.
@@ -1897,8 +1899,7 @@ GDALDataset *JPGDatasetCommon::InitEXIFOverview()
 
     // Read number of entry in directory.
     GUInt16 nEntryCount = 0;
-    if (nTiffDirStart > (INT_MAX - nTIFFHEADER) ||
-        VSIFSeekL(m_fpImage, nTiffDirStart + nTIFFHEADER, SEEK_SET) != 0 ||
+    if (VSIFSeekL(m_fpImage, nTiffDirStart + nTIFFHEADER, SEEK_SET) != 0 ||
         VSIFReadL(&nEntryCount, 1, sizeof(GUInt16), m_fpImage) !=
             sizeof(GUInt16))
     {
@@ -1930,7 +1931,7 @@ GDALDataset *JPGDatasetCommon::InitEXIFOverview()
         return nullptr;
     if (bSwabflag)
         CPL_SWAP32PTR(&nNextDirOff);
-    if (nNextDirOff == 0 || nNextDirOff > UINT_MAX - nTIFFHEADER)
+    if (nNextDirOff == 0)
         return nullptr;
 
     // Seek to IFD1.
@@ -1939,8 +1940,8 @@ GDALDataset *JPGDatasetCommon::InitEXIFOverview()
             sizeof(GUInt16))
     {
         CPLError(CE_Failure, CPLE_AppDefined,
-                 "Error reading IFD1 Directory count at %d.",
-                 nTIFFHEADER + nNextDirOff);
+                 "Error reading IFD1 Directory count at %" PRIu64 ".",
+                 static_cast<uint64_t>(nTIFFHEADER + nNextDirOff));
         return nullptr;
     }
 
@@ -2012,14 +2013,15 @@ GDALDataset *JPGDatasetCommon::InitEXIFOverview()
     }
     if (nCompression != 6 || nImageWidth >= nRasterXSize ||
         nImageHeight >= nRasterYSize || nJpegIFOffset == 0 ||
-        nJpegIFOffset > UINT_MAX - nTIFFHEADER ||
+        nTIFFHEADER > UINT_MAX || nJpegIFOffset > UINT_MAX - nTIFFHEADER ||
         static_cast<int>(nJpegIFByteCount) <= 0)
     {
         return nullptr;
     }
 
     const char *pszSubfile =
-        CPLSPrintf("JPEG_SUBFILE:%u,%d,%s", nTIFFHEADER + nJpegIFOffset,
+        CPLSPrintf("JPEG_SUBFILE:%" PRIu64 ",%d,%s",
+                   static_cast<uint64_t>(nTIFFHEADER + nJpegIFOffset),
                    nJpegIFByteCount, GetDescription());
     JPGDatasetOpenArgs sArgs;
     sArgs.pszFilename = pszSubfile;

frmts/jpeg/jpgdataset.h

@@ -171,12 +171,13 @@ class JPGDatasetCommon CPL_NON_FINAL : public GDALPamDataset
     bool bHasReadDJIMetadata = false;
     bool bHasReadImageStructureMetadata = false;
     char **papszMetadata{};
-    int nExifOffset{-1};
-    int nInterOffset{-1};
-    int nGPSOffset{-1};
+    uint32_t nExifOffset{0};
+    uint32_t nInterOffset{0};
+    uint32_t nGPSOffset{0};
     bool bSwabflag{};
-    int nTiffDirStart{-1};
-    int nTIFFHEADER{-1};
+    bool m_bTiffDirStartInit = false;
+    uint32_t nTiffDirStart{0};
+    vsi_l_offset nTIFFHEADER{0};
     bool bHasDoneJpegCreateDecompress{};
     bool bHasDoneJpegStartDecompress{};
 

frmts/jpegxl/jpegxl.cpp

@@ -360,7 +360,7 @@ bool JPEGXLDataset::Open(GDALOpenInfo *poOpenInfo)
 #else
                 const bool bSwab = abyBoxBuffer[4] == 0x49;
 #endif
-                constexpr int nTIFFHEADER = 0;
+                constexpr uint32_t nTIFFHEADER = 0;
                 uint32_t nTiffDirStart;
                 memcpy(&nTiffDirStart, abyBoxBuffer.data() + 8,
                        sizeof(uint32_t));
@@ -371,9 +371,9 @@ bool JPEGXLDataset::Open(GDALOpenInfo *poOpenInfo)
                 VSILFILE *fpEXIF =
                     VSIFileFromMemBuffer(nullptr, abyBoxBuffer.data() + 4,
                                          abyBoxBuffer.size() - 4, false);
-                int nExifOffset = 0;
-                int nInterOffset = 0;
-                int nGPSOffset = 0;
+                uint32_t nExifOffset = 0;
+                uint32_t nInterOffset = 0;
+                uint32_t nGPSOffset = 0;
                 char **papszEXIFMetadata = nullptr;
                 EXIFExtractMetadata(papszEXIFMetadata, fpEXIF, nTiffDirStart,
                                     bSwab, nTIFFHEADER, nExifOffset,

gcore/gdal_cpp_functions.h

@@ -237,9 +237,11 @@ CPLString GDALFindAssociatedFile(const char *pszBasename, const char *pszExt,
                                  CSLConstList papszSiblingFiles, int nFlags);
 
 CPLErr CPL_DLL EXIFExtractMetadata(char **&papszMetadata, void *fpL,
-                                   int nOffset, int bSwabflag, int nTIFFHEADER,
-                                   int &nExifOffset, int &nInterOffset,
-                                   int &nGPSOffset);
+                                   uint32_t nOffset, int bSwabflag,
+                                   vsi_l_offset nTIFFHEADER,
+                                   uint32_t &nExifOffset,
+                                   uint32_t &nInterOffset,
+                                   uint32_t &nGPSOffset);
 
 int GDALValidateOpenOptions(GDALDriverH hDriver,
                             const char *const *papszOptionOptions);

gcore/gdalexif.cpp

@@ -495,18 +495,18 @@ static int EXIF_TIFFDataWidth(int /* GDALEXIFTIFFDataType */ type)
 /*                                                                      */
 /*      Extract all entry from a IFD                                    */
 /************************************************************************/
-CPLErr EXIFExtractMetadata(char **&papszMetadata, void *fpInL, int nOffset,
-                           int bSwabflag, int nTIFFHEADER, int &nExifOffset,
-                           int &nInterOffset, int &nGPSOffset)
+CPLErr EXIFExtractMetadata(char **&papszMetadata, void *fpInL, uint32_t nOffset,
+                           int bSwabflag, vsi_l_offset nTIFFHEADER,
+                           uint32_t &nExifOffset, uint32_t &nInterOffset,
+                           uint32_t &nGPSOffset)
 {
     /* -------------------------------------------------------------------- */
     /*      Read number of entry in directory                               */
     /* -------------------------------------------------------------------- */
     GUInt16 nEntryCount;
     VSILFILE *const fp = static_cast<VSILFILE *>(fpInL);
 
-    if (nOffset > INT_MAX - nTIFFHEADER ||
-        VSIFSeekL(fp, nOffset + nTIFFHEADER, SEEK_SET) != 0 ||
+    if (VSIFSeekL(fp, nOffset + nTIFFHEADER, SEEK_SET) != 0 ||
         VSIFReadL(&nEntryCount, 1, sizeof(GUInt16), fp) != sizeof(GUInt16))
     {
         CPLError(CE_Failure, CPLE_AppDefined,
@@ -639,25 +639,20 @@ CPLErr EXIFExtractMetadata(char **&papszMetadata, void *fpInL, int nOffset,
         /*      Save important directory tag offset */
         /* --------------------------------------------------------------------
          */
-
-        // Our current API uses int32 and not uint32
-        if (poTIFFDirEntry->tdir_offset < INT_MAX)
+        if (poTIFFDirEntry->tdir_tag == EXIFOFFSETTAG)
         {
-            if (poTIFFDirEntry->tdir_tag == EXIFOFFSETTAG)
-            {
-                nExifOffset = poTIFFDirEntry->tdir_offset;
-                continue;
-            }
-            else if (poTIFFDirEntry->tdir_tag == INTEROPERABILITYOFFSET)
-            {
-                nInterOffset = poTIFFDirEntry->tdir_offset;
-                continue;
-            }
-            else if (poTIFFDirEntry->tdir_tag == GPSOFFSETTAG)
-            {
-                nGPSOffset = poTIFFDirEntry->tdir_offset;
-                continue;
-            }
+            nExifOffset = poTIFFDirEntry->tdir_offset;
+            continue;
+        }
+        else if (poTIFFDirEntry->tdir_tag == INTEROPERABILITYOFFSET)
+        {
+            nInterOffset = poTIFFDirEntry->tdir_offset;
+            continue;
+        }
+        else if (poTIFFDirEntry->tdir_tag == GPSOFFSETTAG)
+        {
+            nGPSOffset = poTIFFDirEntry->tdir_offset;
+            continue;
         }
 
         /* ----------------------------------------------------------------- */