Skip to content

Commit a183403

Browse files
rouaultrouault
authored
Merge pull request OSGeo#11944 from rouault/fix_ossfuzz_402248411
OGR SQL: prevent building too deep expression that can cause stack overflow when destroying them
2 parents dd6009a + f25ef28 commit a183403

File tree

6 files changed

+541
-29
lines changed

6 files changed

+541
-29
lines changed

‎autotest/ogr/ogr_sql_test.py

+70
Original file line numberDiff line numberDiff line change
@@ -2154,3 +2154,73 @@ def test_ogr_sql_kahan_babuska_eumaier_summation(input, expected_output):
21542154
assert math.isnan(f["SUM_v"])
21552155
else:
21562156
assert f["SUM_v"] == expected_output
2157+
2158+
2159+
@pytest.mark.parametrize(
2160+
"operator", ["+", "-", "*", "/", "%", "<", "<=", "=<", "=", "<>", ">", ">=", "=>"]
2161+
)
2162+
def test_ogr_sql_max_expr_depth(operator):
2163+
2164+
ds = ogr.GetDriverByName("Memory").CreateDataSource("")
2165+
ds.CreateLayer("test")
2166+
with ds.ExecuteSQL("SELECT " + operator.join(["1"] * 127) + " FROM test") as _:
2167+
pass
2168+
with pytest.raises(Exception, match="Maximum expression depth reached"):
2169+
ds.ExecuteSQL("SELECT " + operator.join(["1"] * 128) + " FROM test")
2170+
2171+
2172+
def test_ogr_sql_max_expr_depth_other():
2173+
ds = ogr.GetDriverByName("Memory").CreateDataSource("")
2174+
ds.CreateLayer("test")
2175+
2176+
with ds.ExecuteSQL(
2177+
"SELECT CAST(" + "+".join(["1"] * 126) + " AS CHARACTER) FROM test"
2178+
) as _:
2179+
pass
2180+
with pytest.raises(Exception, match="Maximum expression depth reached"):
2181+
ds.ExecuteSQL(
2182+
"SELECT CAST(" + "+".join(["1"] * 127) + " AS CHARACTER) FROM test"
2183+
)
2184+
2185+
with ds.ExecuteSQL(
2186+
"SELECT 'a' IN (CAST(" + "+".join(["1"] * 125) + " AS CHARACTER)) FROM test"
2187+
) as _:
2188+
pass
2189+
with pytest.raises(Exception, match="Maximum expression depth reached"):
2190+
ds.ExecuteSQL(
2191+
"SELECT 'a' IN (CAST(" + "+".join(["1"] * 126) + " AS CHARACTER)) FROM test"
2192+
)
2193+
2194+
with ds.ExecuteSQL("SELECT NOT " + "+".join(["1"] * 126) + " FROM test") as _:
2195+
pass
2196+
with pytest.raises(Exception, match="Maximum expression depth reached"):
2197+
ds.ExecuteSQL("SELECT NOT " + "+".join(["1"] * 127) + " FROM test")
2198+
2199+
with ds.ExecuteSQL("SELECT 1 AND " + "+".join(["1"] * 126) + " FROM test") as _:
2200+
pass
2201+
with pytest.raises(Exception, match="Maximum expression depth reached"):
2202+
ds.ExecuteSQL("SELECT 1 AND " + "+".join(["1"] * 127) + " FROM test")
2203+
2204+
with ds.ExecuteSQL("SELECT 1 OR " + "+".join(["1"] * 126) + " FROM test") as _:
2205+
pass
2206+
with pytest.raises(Exception, match="Maximum expression depth reached"):
2207+
ds.ExecuteSQL("SELECT 1 OR " + "+".join(["1"] * 127) + " FROM test")
2208+
2209+
with ds.ExecuteSQL("SELECT " + "+".join(["1"] * 126) + " IS NULL FROM test") as _:
2210+
pass
2211+
with pytest.raises(Exception, match="Maximum expression depth reached"):
2212+
ds.ExecuteSQL("SELECT " + "+".join(["1"] * 127) + " IS NULL FROM test")
2213+
2214+
with ds.ExecuteSQL(
2215+
"SELECT " + "+".join(["1"] * 125) + " IS NOT NULL FROM test"
2216+
) as _:
2217+
pass
2218+
with pytest.raises(Exception, match="Maximum expression depth reached"):
2219+
ds.ExecuteSQL("SELECT " + "+".join(["1"] * 126) + " IS NOT NULL FROM test")
2220+
2221+
with ds.ExecuteSQL(
2222+
"SELECT SUBSTR('a', " + "+".join(["1"] * 126) + ") FROM test"
2223+
) as _:
2224+
pass
2225+
with pytest.raises(Exception, match="Maximum expression depth reached"):
2226+
ds.ExecuteSQL("SELECT SUBSTR('a', " + "+".join(["1"] * 127) + ") FROM test")

‎ogr/CMakeLists.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ add_custom_target(check_swq_parser_md5 ALL
202202
COMMAND ${CMAKE_COMMAND}
203203
"-DIN_FILE=swq_parser.y"
204204
"-DTARGET=generate_swq_parser"
205-
"-DEXPECTED_MD5SUM=47b674ecca1fc71a50bc81d7d0bfa29d "
205+
"-DEXPECTED_MD5SUM=c3eaca734f3005e73586cc697e481f94"
206206
"-DFILENAME_CMAKE=${CMAKE_CURRENT_SOURCE_DIR}/CMakeLists.txt"
207207
-P "${PROJECT_SOURCE_DIR}/cmake/helpers/check_md5sum.cmake"
208208
WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"

‎ogr/ogr_swq.h

+6-2
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,7 @@ class CPL_UNSTABLE_API swq_expr_node
157157
void Dump(FILE *fp, int depth);
158158
swq_field_type Check(swq_field_list *, int bAllowFieldsInSecondaryTables,
159159
int bAllowMismatchTypeOnFieldComparison,
160-
swq_custom_func_registrar *poCustomFuncRegistrar,
161-
int depth = 0);
160+
swq_custom_func_registrar *poCustomFuncRegistrar);
162161
swq_expr_node *Evaluate(swq_field_fetcher pfnFetcher, void *record,
163162
const swq_evaluation_context &sContext);
164163
swq_expr_node *Clone();
@@ -169,6 +168,8 @@ class CPL_UNSTABLE_API swq_expr_node
169168

170169
void RebalanceAndOr();
171170

171+
bool HasReachedMaxDepth() const;
172+
172173
swq_node_type eNodeType = SNT_CONSTANT;
173174
swq_field_type field_type = SWQ_INTEGER;
174175

@@ -199,6 +200,9 @@ class CPL_UNSTABLE_API swq_expr_node
199200
// after parsing. swq_col_def.bHidden captures it afterwards.
200201
bool bHidden = false;
201202

203+
// Recursive depth of this expression, taking into account papoSubExpr.
204+
int nDepth = 1;
205+
202206
static CPLString QuoteIfNecessary(const CPLString &, char chQuote = '\'');
203207
static CPLString Quote(const CPLString &, char chQuote = '\'');
204208
};

‎ogr/swq_expr_node.cpp

+19-13
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,7 @@ swq_expr_node &swq_expr_node::operator=(const swq_expr_node &other)
222222
if (other.string_value)
223223
string_value = CPLStrdup(other.string_value);
224224
bHidden = other.bHidden;
225+
nDepth = other.nDepth;
225226
}
226227
return *this;
227228
}
@@ -258,6 +259,7 @@ swq_expr_node &swq_expr_node::operator=(swq_expr_node &&other)
258259
std::swap(geometry_value, other.geometry_value);
259260
std::swap(string_value, other.string_value);
260261
bHidden = other.bHidden;
262+
nDepth = other.nDepth;
261263
}
262264
return *this;
263265
}
@@ -286,6 +288,8 @@ void swq_expr_node::PushSubExpression(swq_expr_node *child)
286288
CPLRealloc(papoSubExpr, sizeof(void *) * nSubExprCount));
287289

288290
papoSubExpr[nSubExprCount - 1] = child;
291+
292+
nDepth = std::max(nDepth, 1 + child->nDepth);
289293
}
290294

291295
/************************************************************************/
@@ -307,19 +311,13 @@ void swq_expr_node::ReverseSubExpressions()
307311
/* Check argument types, etc. */
308312
/************************************************************************/
309313

310-
swq_field_type swq_expr_node::Check(
311-
swq_field_list *poFieldList, int bAllowFieldsInSecondaryTables,
312-
int bAllowMismatchTypeOnFieldComparison,
313-
swq_custom_func_registrar *poCustomFuncRegistrar, int nDepth)
314+
swq_field_type
315+
swq_expr_node::Check(swq_field_list *poFieldList,
316+
int bAllowFieldsInSecondaryTables,
317+
int bAllowMismatchTypeOnFieldComparison,
318+
swq_custom_func_registrar *poCustomFuncRegistrar)
314319

315320
{
316-
if (nDepth == 32)
317-
{
318-
CPLError(CE_Failure, CPLE_AppDefined,
319-
"Too many recursion levels in expression");
320-
return SWQ_ERROR;
321-
}
322-
323321
/* -------------------------------------------------------------------- */
324322
/* Otherwise we take constants literally. */
325323
/* -------------------------------------------------------------------- */
@@ -390,8 +388,7 @@ swq_field_type swq_expr_node::Check(
390388
{
391389
if (papoSubExpr[i]->Check(poFieldList, bAllowFieldsInSecondaryTables,
392390
bAllowMismatchTypeOnFieldComparison,
393-
poCustomFuncRegistrar,
394-
nDepth + 1) == SWQ_ERROR)
391+
poCustomFuncRegistrar) == SWQ_ERROR)
395392
return SWQ_ERROR;
396393
}
397394

@@ -1163,4 +1160,13 @@ void swq_expr_node::PushNotOperationDownToStack()
11631160
papoSubExpr[i]->PushNotOperationDownToStack();
11641161
}
11651162

1163+
/************************************************************************/
1164+
/* HasReachedMaxDepth() */
1165+
/************************************************************************/
1166+
1167+
bool swq_expr_node::HasReachedMaxDepth() const
1168+
{
1169+
return nDepth == 128;
1170+
}
1171+
11661172
#endif // #ifndef DOXYGEN_SKIP

0 commit comments

Comments
 (0)