Skip to content

Commit 4e39697

Browse files
committed
fix(sidecar): allowlist band-math AST and assert Sedona limit
Reject bytes/tuple allocations before eval, and lock the SQL overflow test to MAX_FEATURES + 1 so the bound cannot drift silently.
1 parent 70a277d commit 4e39697

3 files changed

Lines changed: 78 additions & 28 deletions

File tree

backend/geolibre_server/geolibre_server/app/raster.py

Lines changed: 39 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,46 +1027,59 @@ def load_bands(path, letter, namespace, base):
10271027
namespace.update(safe_funcs)
10281028
# Band arrays are full ndarrays, so attribute access (``A.tofile(...)``,
10291029
# ``A.dump(...)``) would still write files even with an empty ``__builtins__``
1030-
# and no bare ``np``. Parse the expression and reject attribute access plus any
1031-
# call that is not one of the curated safe functions before ``eval``.
1030+
# and no bare ``np``. Allowlist the expression AST before ``eval`` so only
1031+
# curated calls/operators run — and so list/bytes/tuple multiplication cannot
1032+
# allocate unbounded memory before the shape check.
10321033
try:
10331034
tree = ast.parse(expression, mode="eval")
10341035
except SyntaxError as exc:
10351036
raise SystemExit(f"Failed to evaluate expression: {exc}") from exc
10361037
allowed_calls = set(safe_funcs)
1038+
_allowed_nodes = (
1039+
ast.Expression,
1040+
ast.Name,
1041+
ast.Load,
1042+
ast.Constant,
1043+
ast.Call,
1044+
ast.BinOp,
1045+
ast.UnaryOp,
1046+
ast.BoolOp,
1047+
ast.Compare,
1048+
ast.IfExp,
1049+
ast.Add,
1050+
ast.Sub,
1051+
ast.Mult,
1052+
ast.Div,
1053+
ast.FloorDiv,
1054+
ast.Mod,
1055+
ast.Pow,
1056+
ast.UAdd,
1057+
ast.USub,
1058+
ast.Not,
1059+
ast.And,
1060+
ast.Or,
1061+
ast.Eq,
1062+
ast.NotEq,
1063+
ast.Lt,
1064+
ast.LtE,
1065+
ast.Gt,
1066+
ast.GtE,
1067+
ast.keyword,
1068+
)
10371069
for node in ast.walk(tree):
1038-
if isinstance(node, ast.Attribute):
1070+
if not isinstance(node, _allowed_nodes):
10391071
raise SystemExit(
1040-
"Expression may not access attributes "
1072+
f"Expression may not use {type(node).__name__} "
10411073
"(band math only allows curated functions and operators)"
10421074
)
1043-
# Reject containers/comprehensions so expressions like ``[0] * 10**9``
1044-
# cannot allocate unbounded memory before the shape check runs.
1045-
if isinstance(
1046-
node,
1047-
(
1048-
ast.List,
1049-
ast.Dict,
1050-
ast.Set,
1051-
ast.ListComp,
1052-
ast.DictComp,
1053-
ast.SetComp,
1054-
ast.GeneratorExp,
1055-
),
1056-
):
1075+
if isinstance(node, ast.Constant) and not isinstance(node.value, (int, float, bool)):
10571076
raise SystemExit(
1058-
"Expression may not build lists, dicts, or comprehensions "
1077+
"Expression may only use numeric constants "
10591078
"(band math only allows curated functions and operators)"
10601079
)
10611080
if isinstance(node, ast.Call):
10621081
# ``np.where(...)`` / ``A.tofile(...)`` parse as Attribute-backed calls;
1063-
# reject them the same way as bare attribute access so ndarray file I/O
1064-
# and the withheld ``np`` module stay unreachable.
1065-
if isinstance(node.func, ast.Attribute):
1066-
raise SystemExit(
1067-
"Expression may not access attributes "
1068-
"(band math only allows curated functions and operators)"
1069-
)
1082+
# Attribute is already rejected above, but keep an explicit Name check.
10701083
if not isinstance(node.func, ast.Name) or node.func.id not in allowed_calls:
10711084
name = node.func.id if isinstance(node.func, ast.Name) else type(node.func).__name__
10721085
raise SystemExit(f"Call to '{name}' is not allowed in band math")

backend/geolibre_server/tests/test_raster.py

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,7 @@ def test_raster_calculator_blocks_numpy_io(tmp_path: Path) -> None:
637637
combined = completed.stdout + completed.stderr
638638
assert (
639639
"Failed to evaluate expression" in combined
640+
or "may not use" in combined
640641
or "may not access attributes" in combined
641642
or "not allowed in band math" in combined
642643
)
@@ -1056,6 +1057,39 @@ def test_raster_calculator_blocks_ndarray_tofile(tmp_path: Path) -> None:
10561057
)
10571058
assert completed.returncode != 0
10581059
combined = completed.stdout + completed.stderr
1059-
assert "may not access attributes" in combined
1060+
assert "may not use" in combined or "may not access attributes" in combined
10601061
assert not evil.exists()
10611062
assert not out.exists()
1063+
1064+
1065+
@requires_rasterio
1066+
def test_raster_calculator_rejects_unbounded_allocations(tmp_path: Path) -> None:
1067+
"""List/bytes/tuple multiplication must not allocate before the shape check."""
1068+
src = _write_dem(tmp_path / "dem.tif")
1069+
out = tmp_path / "calc.tif"
1070+
for expression in (
1071+
"[0] * 10**9",
1072+
'b"x" * 10**9',
1073+
"(0,) * 10**9",
1074+
):
1075+
completed = subprocess.run(
1076+
[
1077+
sys.executable,
1078+
"-c",
1079+
_RASTER_TOOL_SCRIPTS["raster-calc"],
1080+
json.dumps(
1081+
{
1082+
"input_path": str(src),
1083+
"output_path": str(out),
1084+
"expression": expression,
1085+
}
1086+
),
1087+
],
1088+
check=False,
1089+
capture_output=True,
1090+
text=True,
1091+
)
1092+
assert completed.returncode != 0, expression
1093+
combined = completed.stdout + completed.stderr
1094+
assert "may not use" in combined or "numeric constants" in combined, expression
1095+
assert not out.exists()

backend/geolibre_server/tests/test_sql.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ def test_run_rejects_oversized_layer(monkeypatch: pytest.MonkeyPatch) -> None:
127127
def test_run_rejects_oversized_result(monkeypatch: pytest.MonkeyPatch) -> None:
128128
"""Query results that expand past MAX_FEATURES are refused with 413."""
129129
monkeypatch.setattr(sedona_ops, "MAX_FEATURES", 1)
130+
limited_to: list[int] = []
130131

131132
class _FakeFrame:
132133
def __len__(self) -> int:
@@ -140,7 +141,8 @@ def to_dict(self, orient: str): # noqa: ARG002
140141
return [{"n": 1}, {"n": 2}]
141142

142143
class _FakeResult:
143-
def limit(self, n: int): # noqa: ARG002
144+
def limit(self, n: int) -> "_FakeResult":
145+
limited_to.append(n)
144146
return self
145147

146148
def to_pandas(self):
@@ -169,3 +171,4 @@ def close(self) -> None:
169171
sql_run(SqlRunRequest(sql="SELECT 1 AS n UNION ALL SELECT 2 AS n"))
170172
assert exc.value.status_code == 413
171173
assert "Query result exceeds" in str(exc.value.detail)
174+
assert limited_to == [sedona_ops.MAX_FEATURES + 1]

0 commit comments

Comments
 (0)