Skip to content

Commit

Permalink
OGR SQL: prevent building too deep expression that can cause stack ov…
Browse files Browse the repository at this point in the history
…erflow when destroying them

Fixes https://issues.oss-fuzz.com/issues/402248411
  • Loading branch information
rouault committed Mar 11, 2025
1 parent fdfe753 commit 9158681
Show file tree
Hide file tree
Showing 6 changed files with 540 additions and 32 deletions.
70 changes: 70 additions & 0 deletions autotest/ogr/ogr_sql_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2154,3 +2154,73 @@ def test_ogr_sql_kahan_babuska_eumaier_summation(input, expected_output):
assert math.isnan(f["SUM_v"])
else:
assert f["SUM_v"] == expected_output


@pytest.mark.parametrize(
"operator", ["+", "-", "*", "/", "%", "<", "<=", "=<", "=", "<>", ">", ">=", "=>"]
)
def test_ogr_sql_max_expr_depth(operator):

ds = ogr.GetDriverByName("Memory").CreateDataSource("")
ds.CreateLayer("test")
with ds.ExecuteSQL("SELECT " + operator.join(["1"] * 127) + " FROM test") as _:
pass
with pytest.raises(Exception, match="Maximum expression depth reached"):
ds.ExecuteSQL("SELECT " + operator.join(["1"] * 128) + " FROM test")


def test_ogr_sql_max_expr_depth_other():
ds = ogr.GetDriverByName("Memory").CreateDataSource("")
ds.CreateLayer("test")

with ds.ExecuteSQL(
"SELECT CAST(" + "+".join(["1"] * 126) + " AS CHARACTER) FROM test"
) as _:
pass
with pytest.raises(Exception, match="Maximum expression depth reached"):
ds.ExecuteSQL(
"SELECT CAST(" + "+".join(["1"] * 127) + " AS CHARACTER) FROM test"
)

with ds.ExecuteSQL(
"SELECT 'a' IN (CAST(" + "+".join(["1"] * 125) + " AS CHARACTER)) FROM test"
) as _:
pass
with pytest.raises(Exception, match="Maximum expression depth reached"):
ds.ExecuteSQL(
"SELECT 'a' IN (CAST(" + "+".join(["1"] * 126) + " AS CHARACTER)) FROM test"
)

with ds.ExecuteSQL("SELECT NOT " + "+".join(["1"] * 126) + " FROM test") as _:
pass
with pytest.raises(Exception, match="Maximum expression depth reached"):
ds.ExecuteSQL("SELECT NOT " + "+".join(["1"] * 127) + " FROM test")

with ds.ExecuteSQL("SELECT 1 AND " + "+".join(["1"] * 126) + " FROM test") as _:
pass
with pytest.raises(Exception, match="Maximum expression depth reached"):
ds.ExecuteSQL("SELECT 1 AND " + "+".join(["1"] * 127) + " FROM test")

with ds.ExecuteSQL("SELECT 1 OR " + "+".join(["1"] * 126) + " FROM test") as _:
pass
with pytest.raises(Exception, match="Maximum expression depth reached"):
ds.ExecuteSQL("SELECT 1 OR " + "+".join(["1"] * 127) + " FROM test")

with ds.ExecuteSQL("SELECT " + "+".join(["1"] * 126) + " IS NULL FROM test") as _:
pass
with pytest.raises(Exception, match="Maximum expression depth reached"):
ds.ExecuteSQL("SELECT " + "+".join(["1"] * 127) + " IS NULL FROM test")

with ds.ExecuteSQL(
"SELECT " + "+".join(["1"] * 125) + " IS NOT NULL FROM test"
) as _:
pass
with pytest.raises(Exception, match="Maximum expression depth reached"):
ds.ExecuteSQL("SELECT " + "+".join(["1"] * 126) + " IS NOT NULL FROM test")

with ds.ExecuteSQL(
"SELECT SUBSTR('a', " + "+".join(["1"] * 126) + ") FROM test"
) as _:
pass
with pytest.raises(Exception, match="Maximum expression depth reached"):
ds.ExecuteSQL("SELECT SUBSTR('a', " + "+".join(["1"] * 127) + ") FROM test")
2 changes: 1 addition & 1 deletion ogr/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ add_custom_target(check_swq_parser_md5 ALL
COMMAND ${CMAKE_COMMAND}
"-DIN_FILE=swq_parser.y"
"-DTARGET=generate_swq_parser"
"-DEXPECTED_MD5SUM=47b674ecca1fc71a50bc81d7d0bfa29d "
"-DEXPECTED_MD5SUM=c3eaca734f3005e73586cc697e481f94"
"-DFILENAME_CMAKE=${CMAKE_CURRENT_SOURCE_DIR}/CMakeLists.txt"
-P "${PROJECT_SOURCE_DIR}/cmake/helpers/check_md5sum.cmake"
WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
Expand Down
8 changes: 6 additions & 2 deletions ogr/ogr_swq.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,7 @@ class CPL_UNSTABLE_API swq_expr_node
void Dump(FILE *fp, int depth);
swq_field_type Check(swq_field_list *, int bAllowFieldsInSecondaryTables,
int bAllowMismatchTypeOnFieldComparison,
swq_custom_func_registrar *poCustomFuncRegistrar,
int depth = 0);
swq_custom_func_registrar *poCustomFuncRegistrar);
swq_expr_node *Evaluate(swq_field_fetcher pfnFetcher, void *record,
const swq_evaluation_context &sContext);
swq_expr_node *Clone();
Expand All @@ -169,6 +168,8 @@ class CPL_UNSTABLE_API swq_expr_node

void RebalanceAndOr();

bool HasReachedMaxDepth() const;

swq_node_type eNodeType = SNT_CONSTANT;
swq_field_type field_type = SWQ_INTEGER;

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

// Recursive depth of this expression, taking into account papoSubExpr.
int nDepth = 1;

static CPLString QuoteIfNecessary(const CPLString &, char chQuote = '\'');
static CPLString Quote(const CPLString &, char chQuote = '\'');
};
Expand Down
30 changes: 17 additions & 13 deletions ogr/swq_expr_node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,8 @@ void swq_expr_node::PushSubExpression(swq_expr_node *child)
CPLRealloc(papoSubExpr, sizeof(void *) * nSubExprCount));

papoSubExpr[nSubExprCount - 1] = child;

nDepth = std::max(nDepth, 1 + child->nDepth);
}

/************************************************************************/
Expand All @@ -307,19 +309,13 @@ void swq_expr_node::ReverseSubExpressions()
/* Check argument types, etc. */
/************************************************************************/

swq_field_type swq_expr_node::Check(
swq_field_list *poFieldList, int bAllowFieldsInSecondaryTables,
int bAllowMismatchTypeOnFieldComparison,
swq_custom_func_registrar *poCustomFuncRegistrar, int nDepth)
swq_field_type
swq_expr_node::Check(swq_field_list *poFieldList,
int bAllowFieldsInSecondaryTables,
int bAllowMismatchTypeOnFieldComparison,
swq_custom_func_registrar *poCustomFuncRegistrar)

{
if (nDepth == 32)
{
CPLError(CE_Failure, CPLE_AppDefined,
"Too many recursion levels in expression");
return SWQ_ERROR;
}

/* -------------------------------------------------------------------- */
/* Otherwise we take constants literally. */
/* -------------------------------------------------------------------- */
Expand Down Expand Up @@ -390,8 +386,7 @@ swq_field_type swq_expr_node::Check(
{
if (papoSubExpr[i]->Check(poFieldList, bAllowFieldsInSecondaryTables,
bAllowMismatchTypeOnFieldComparison,
poCustomFuncRegistrar,
nDepth + 1) == SWQ_ERROR)
poCustomFuncRegistrar) == SWQ_ERROR)
return SWQ_ERROR;
}

Expand Down Expand Up @@ -1163,4 +1158,13 @@ void swq_expr_node::PushNotOperationDownToStack()
papoSubExpr[i]->PushNotOperationDownToStack();
}

/************************************************************************/
/* HasReachedMaxDepth() */
/************************************************************************/

bool swq_expr_node::HasReachedMaxDepth() const
{
return nDepth == 128;
}

#endif // #ifndef DOXYGEN_SKIP
Loading

0 comments on commit 9158681

Please sign in to comment.