VRT expressions: Enforce maximum length and nesting depth

OSGeo · Nov 14, 2024 · 19570d6 · 19570d6
1 parent ed2b001
commit 19570d6
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 12 deletions.
diff --git a/autotest/gdrivers/vrtderived.py b/autotest/gdrivers/vrtderived.py
@@ -1178,26 +1178,45 @@ def test_vrt_pixelfn_expression(tmp_path, expression, sources, result):
         assert pytest.approx(ds.ReadAsArray()[0][0], nan_ok=True) == result
 
 
-@gdaltest.enable_exceptions()
 @pytest.mark.parametrize(
     "expression,sources,exception",
     [
         pytest.param(
             "A*B + C",
             [("A", 77), ("B", 63)],
-            "Failed to parse expression",
+            "Undefined symbol",
             id="undefined variable",
         ),
+        pytest.param(
+            "(".join(["asin", "sin", "acos", "cos"] * 100) + "(X" + 100 * 4 * ")",
+            [("X", 0.5)],
+            "exceeds maximum allowed stack depth",
+            id="expression is too complex",
+        ),
+        pytest.param(
+            " ".join(["sin(x) + cos(x)"] * 10000),
+            [("x", 0.5)],
+            "exceeds maximum of 100000 set by GDAL_EXPRTK_MAX_EXPRESSION_LENGTH",
+            id="expression is too long",
+        ),
     ],
 )
 def test_vrt_pixelfn_expression_invalid(tmp_path, expression, sources, exception):
     pytest.importorskip("numpy")
 
+    messages = []
+
+    def handle(ecls, ecode, emsg):
+        messages.append(emsg)
+
     xml = vrt_expression_xml(tmp_path, expression, sources)
 
-    with gdal.Open(xml) as ds:
-        with pytest.raises(Exception, match=exception):
-            ds.ReadAsArray()
+    with gdaltest.error_handler(handle):
+        ds = gdal.Open(xml)
+        if ds:
+            assert ds.ReadAsArray() is None
+
+    assert exception in "".join(messages)
 
 
 ###############################################################################

diff --git a/autotest/gdrivers/vrtprocesseddataset.py b/autotest/gdrivers/vrtprocesseddataset.py
@@ -1223,8 +1223,14 @@ def test_vrtprocesseddataset_trimming_errors(tmp_vsimem):
         ),
         pytest.param(
             """
-             var chunk[10];
-             var out[5];
+             // Reduce every 10 bands of input into a single
+             // band of output, using avg()
+             const var chunksize := 10;
+             const var outsize := ALL_BANDS[] / chunksize;
+
+             var chunk[chunksize];
+             var out[outsize];
+
              for (var i := 0; i < out[]; i += 1) {
                 for (var j := 0; j < chunk[]; j += 1) {
                     chunk[j] := ALL_BANDS[i * chunk[] + j];
@@ -1301,7 +1307,7 @@ def test_vrtprocesseddataset_trimming_errors(tmp_vsimem):
             np.array([[[1, 2]]]),
             np.array([[[1, 1]]]),
             "Failed to parse expression",
-            {"GDAL_EXPRTK_MAX_VECTOR_SIZE": "4"},
+            {"GDAL_EXPRTK_MAX_VECTOR_LENGTH": "4"},
             id="vector too large",
         ),
         pytest.param(

diff --git a/frmts/vrt/vrtexpression.cpp b/frmts/vrt/vrtexpression.cpp
@@ -65,12 +65,12 @@ class GDALExpressionEvaluator::Impl
 
         m_oParser.register_vector_access_runtime_check(m_oVectorAccessCheck);
 
-        int nMaxVectorSize = std::atoi(
-            CPLGetConfigOption("GDAL_EXPRTK_MAX_VECTOR_SIZE", "100000"));
+        int nMaxVectorLength = std::atoi(
+            CPLGetConfigOption("GDAL_EXPRTK_MAX_VECTOR_LENGTH", "100000"));
 
-        if (nMaxVectorSize > 0)
+        if (nMaxVectorLength > 0)
         {
-            m_oParser.settings().set_max_local_vector_size(nMaxVectorSize);
+            m_oParser.settings().set_max_local_vector_size(nMaxVectorLength);
         }
 
         bool bEnableLoops =
@@ -88,6 +88,19 @@ class GDALExpressionEvaluator::Impl
 
     CPLErr compile()
     {
+        int nMaxExpressionLength = std::atoi(
+            CPLGetConfigOption("GDAL_EXPRTK_MAX_EXPRESSION_LENGTH", "100000"));
+        if (m_osExpression.size() >
+            static_cast<std::size_t>(nMaxExpressionLength))
+        {
+            CPLError(CE_Failure, CPLE_AppDefined,
+                     "Expression length of %d exceeds maximum of %d set by "
+                     "GDAL_EXPRTK_MAX_EXPRESSION_LENGTH",
+                     static_cast<int>(m_osExpression.size()),
+                     nMaxExpressionLength);
+            return CE_Failure;
+        }
+
         for (const auto &[osVariable, pdfValueLoc] : m_aoVariables)
         {
             m_oSymbolTable.add_variable(osVariable, *pdfValueLoc);