Merge pull request #10594 from OSGeo/backport-10587-to-release/3.9

[Backport release/3.9] netCDF multidim: fix use-after-free on string variables in ReadOneElement()
OSGeo · Aug 13, 2024 · d2d00c1 · d2d00c1
2 parents 09e8d86 + 817153c
commit d2d00c1
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 22 deletions.
diff --git a/autotest/gdrivers/netcdf_multidim.py b/autotest/gdrivers/netcdf_multidim.py
@@ -1250,6 +1250,15 @@ def dims_from_non_netcdf(rg):
         assert var
         assert var.Read() == ["", "0123456789"]
 
+        var = rg.CreateMDArray(
+            "my_var_string_array_zero_dim", [], gdal.ExtendedDataType.CreateString()
+        )
+        assert var
+        assert var.Write(["foo"]) == gdal.CE_None
+        var = rg.OpenMDArray("my_var_string_array_zero_dim")
+        assert var
+        assert var.Read() == ["foo"]
+
     f()
 
     def f2():

diff --git a/frmts/netcdf/netcdfmultidim.cpp b/frmts/netcdf/netcdfmultidim.cpp
@@ -3403,9 +3403,9 @@ bool netCDFVariable::ReadOneElement(const GDALExtendedDataType &src_datatype,
         NCDF_ERR(ret);
         if (ret != NC_NOERR)
             return false;
-        nc_free_string(1, &pszStr);
         GDALExtendedDataType::CopyValue(&pszStr, src_datatype, pDstBuffer,
                                         bufferDataType);
+        nc_free_string(1, &pszStr);
         return true;
     }
 

diff --git a/swig/include/MultiDimensional.i b/swig/include/MultiDimensional.i
@@ -691,8 +691,8 @@ public:
   {
 
     const int nExpectedDims = (int)GDALMDArrayGetDimensionCount(self);
-    std::vector<size_t> count_internal(nExpectedDims);
-    if( nExpectedDims != 1 )
+    std::vector<size_t> count_internal(nExpectedDims + 1);
+    if( nExpectedDims > 1 )
     {
         CPLError(CE_Failure, CPLE_AppDefined,
             "Unsupported number of dimensions");
@@ -707,23 +707,26 @@ public:
             return CE_Failure;
         }
     }
-    if( nDims1 != 1 )
+    if( nExpectedDims == 1 )
     {
-        CPLError(CE_Failure, CPLE_AppDefined,
-            "Wrong number of values in array_start_idx");
-        return CE_Failure;
-    }
-    if( nDims2 != 1 )
-    {
-        CPLError(CE_Failure, CPLE_AppDefined,
-            "Wrong number of values in count");
-        return CE_Failure;
-    }
-    if( nDims3 != 1 )
-    {
-        CPLError(CE_Failure, CPLE_AppDefined,
-            "Wrong number of values in array_step");
-        return CE_Failure;
+        if( nDims1 != 1 )
+        {
+            CPLError(CE_Failure, CPLE_AppDefined,
+                "Wrong number of values in array_start_idx");
+            return CE_Failure;
+        }
+        if( nDims2 != 1 )
+        {
+            CPLError(CE_Failure, CPLE_AppDefined,
+                "Wrong number of values in count");
+            return CE_Failure;
+        }
+        if( nDims3 != 1 )
+        {
+            CPLError(CE_Failure, CPLE_AppDefined,
+                "Wrong number of values in array_step");
+            return CE_Failure;
+        }
     }
 
     CPLErr eErr = GDALMDArrayWrite(self,

diff --git a/swig/include/python/gdal_python.i b/swig/include/python/gdal_python.i
@@ -1857,13 +1857,13 @@ def GetMDArrayNames(self, options = []) -> "list[str]":
       if not buffer_datatype:
         buffer_datatype = self.GetDataType()
 
-      is_1d_string = self.GetDataType().GetClass() == GEDTC_STRING and buffer_datatype.GetClass() == GEDTC_STRING and dimCount == 1
+      is_0d_or_1d_string = self.GetDataType().GetClass() == GEDTC_STRING and buffer_datatype.GetClass() == GEDTC_STRING and dimCount <= 1
 
       if not array_start_idx:
         array_start_idx = [0] * dimCount
 
       if not count:
-        if is_1d_string:
+        if is_0d_or_1d_string:
             assert type(buffer) == type([])
             count = [ len(buffer) ]
         else:
@@ -1882,7 +1882,7 @@ def GetMDArrayNames(self, options = []) -> "list[str]":
             stride *= cnt
         buffer_stride.reverse()
 
-      if is_1d_string:
+      if is_0d_or_1d_string:
           return _gdal.MDArray_WriteStringArray(self, array_start_idx, count, array_step, buffer_datatype, buffer)
 
       return _gdal.MDArray_Write(self, array_start_idx, count, array_step, buffer_stride, buffer_datatype, buffer)