Merge pull request #11561 from rouault/ovr_invalid_factor

gdaladdo / GDALDataset::BuildOverviews(): validate values of decimation factors
OSGeo · Jan 4, 2025 · f889151 · f889151
2 parents 41ffedc + fa8d6a4
commit f889151
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 1 deletion.
diff --git a/apps/gdaladdo.cpp b/apps/gdaladdo.cpp
@@ -692,7 +692,23 @@ MAIN_START(nArgc, papszArgv)
     {
         for (const auto &level : *levels)
         {
+            if (CPLGetValueType(level.c_str()) != CPL_VALUE_INTEGER)
+            {
+                CPLError(
+                    CE_Failure, CPLE_IllegalArg,
+                    "Value '%s' is not a positive integer subsampling factor",
+                    level.c_str());
+                std::exit(1);
+            }
             anLevels.push_back(atoi(level.c_str()));
+            if (anLevels.back() <= 0)
+            {
+                CPLError(
+                    CE_Failure, CPLE_IllegalArg,
+                    "Value '%s' is not a positive integer subsampling factor",
+                    level.c_str());
+                std::exit(1);
+            }
             if (anLevels.back() == 1)
             {
                 printf(

diff --git a/autotest/gcore/tiff_ovr.py b/autotest/gcore/tiff_ovr.py
@@ -165,6 +165,24 @@ def test_tiff_ovr_3(mfloat32_tif, both_endian):
     src_ds = None
 
 
+###############################################################################
+#
+
+
+@gdaltest.enable_exceptions()
+def test_tiff_ovr_invalid_ovr_factor(tmp_path):
+    tif_fname = str(tmp_path / "byte.tif")
+
+    shutil.copyfile("data/byte.tif", tif_fname)
+
+    ds = gdal.Open(tif_fname, gdal.GA_Update)
+    with pytest.raises(
+        Exception,
+        match=r"panOverviewList\[1\] = 0 is invalid\. It must be a positive value",
+    ):
+        ds.BuildOverviews(overviewlist=[2, 0])
+
+
 ###############################################################################
 # Test generation
 

diff --git a/autotest/utilities/test_gdaladdo.py b/autotest/utilities/test_gdaladdo.py
@@ -455,3 +455,33 @@ def test_gdaladdo_partial_refresh_from_source_timestamp_gti(gdaladdo_path, tmp_p
             ovr_data_refreshed[idx] = ovr_data_ori[idx]
     assert ovr_data_refreshed == ovr_data_ori
     ds = None
+
+
+###############################################################################
+#
+
+
+def test_gdaladdo_illegal_factor(gdaladdo_path, tmp_path):
+
+    shutil.copyfile("../gcore/data/byte.tif", f"{tmp_path}/byte.tif")
+
+    _, err = gdaltest.runexternal_out_and_err(
+        f"{gdaladdo_path} -r average {tmp_path}/byte.tif invalid"
+    )
+    assert "Value 'invalid' is not a positive integer subsampling factor" in err
+    with gdal.Open(f"{tmp_path}/byte.tif") as ds:
+        assert ds.GetRasterBand(1).GetOverviewCount() == 0
+
+    _, err = gdaltest.runexternal_out_and_err(
+        f"{gdaladdo_path} -r average {tmp_path}/byte.tif 0"
+    )
+    assert "Value '0' is not a positive integer subsampling factor" in err
+    with gdal.Open(f"{tmp_path}/byte.tif") as ds:
+        assert ds.GetRasterBand(1).GetOverviewCount() == 0
+
+    _, err = gdaltest.runexternal_out_and_err(
+        f"{gdaladdo_path} -r average {tmp_path}/byte.tif -1"
+    )
+    assert "Value '-1' is not a positive integer subsampling factor" in err
+    with gdal.Open(f"{tmp_path}/byte.tif") as ds:
+        assert ds.GetRasterBand(1).GetOverviewCount() == 0
diff --git a/gcore/gdaldataset.cpp b/gcore/gdaldataset.cpp
@@ -2104,7 +2104,8 @@ CPLErr GDALSetGCPs2(GDALDatasetH hDS, int nGCPCount, const GDAL_GCP *pasGCPList,
  * "BILINEAR", "CUBIC", "CUBICSPLINE", "GAUSS", "LANCZOS", "MODE", "NEAREST",
  * or "NONE" controlling the downsampling method applied.
  * @param nOverviews number of overviews to build, or 0 to clean overviews.
- * @param panOverviewList the list of overview decimation factors to build, or
+ * @param panOverviewList the list of overview decimation factors (positive
+ *                        integers, normally larger or equal to 2) to build, or
  *                        NULL if nOverviews == 0.
  * @param nListBands number of bands to build overviews for in panBandList.
  * Build for all bands if this is 0.
@@ -2151,6 +2152,18 @@ CPLErr GDALDataset::BuildOverviews(const char *pszResampling, int nOverviews,
     if (pfnProgress == nullptr)
         pfnProgress = GDALDummyProgress;
 
+    for (int i = 0; i < nOverviews; ++i)
+    {
+        if (panOverviewList[i] <= 0)
+        {
+            CPLError(CE_Failure, CPLE_IllegalArg,
+                     "panOverviewList[%d] = %d is invalid. It must be a "
+                     "positive value",
+                     i, panOverviewList[i]);
+            return CE_Failure;
+        }
+    }
+
     // At time of writing, all overview generation options are actually
     // expected to be passed as configuration options.
     std::vector<std::unique_ptr<CPLConfigOptionSetter>> apoConfigOptionSetter;