Skip to content

Commit 9c1bb56

Browse files
authored
Merge pull request #1293 from kunal-10-cloud/fix/issue-1292-sanitize-eval-query
Fix: Sanitize user input passed to DataFrame.eval()/query() to prevent code injection
2 parents 4a3e521 + 4d8eb65 commit 9c1bb56

File tree

13 files changed

+404
-23
lines changed

13 files changed

+404
-23
lines changed

malariagen_data/anoph/base.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
from numpydoc_decorator import doc # type: ignore
2929
from tqdm.auto import tqdm as tqdm_auto # type: ignore
3030
from tqdm.dask import TqdmCallback # type: ignore
31+
32+
from .safe_query import validate_query
3133
from yaspin import yaspin # type: ignore
3234
import xarray as xr
3335

@@ -980,10 +982,9 @@ def _filter_sample_dataset(
980982

981983
# Determine which samples match the sample query.
982984
if sample_query != "":
983-
# Use the python engine in order to support extension array dtypes, e.g. Float64, Int64, boolean.
984-
loc_samples = df_samples.eval(
985-
sample_query, **sample_query_options, engine="python"
986-
)
985+
# Validate the query to prevent arbitrary code execution (GH-1292).
986+
validate_query(sample_query)
987+
loc_samples = df_samples.eval(sample_query, **sample_query_options)
987988
else:
988989
loc_samples = pd.Series(True, index=df_samples.index)
989990

malariagen_data/anoph/cnv_frq.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
_build_cohorts_from_sample_grouping,
1616
_add_frequency_ci,
1717
)
18+
from .safe_query import validate_query
1819
from ..util import (
1920
_check_types,
2021
_pandas_apply,
@@ -671,6 +672,7 @@ def _gene_cnv_frequencies_advanced(
671672

672673
debug("apply variant query")
673674
if variant_query is not None:
675+
validate_query(variant_query)
674676
loc_variants = df_variants.eval(variant_query).values
675677
# Convert boolean mask to integer indices for NumPy 2.x compatibility
676678
variant_indices = np.where(loc_variants)[0]

malariagen_data/anoph/frq_base.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,10 @@ def _build_cohorts_from_sample_grouping(
147147
period_str = df_cohorts["period"].astype(str)
148148
df_cohorts["label"] = area_str + "_" + taxon_clean + "_" + period_str
149149

150-
# Apply minimum cohort size.
151-
df_cohorts = df_cohorts.query(f"size >= {min_cohort_size}").reset_index(drop=True)
150+
# Apply minimum cohort size using safe boolean indexing.
151+
df_cohorts = df_cohorts.loc[df_cohorts["size"] >= min_cohort_size].reset_index(
152+
drop=True
153+
)
152154

153155
# Early check for no cohorts.
154156
if len(df_cohorts) == 0:

malariagen_data/anoph/genome_features.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,8 @@ def _genome_features_for_contig(self, *, contig: str, attributes: Tuple[str, ...
117117
)
118118
df = self._genome_features(attributes=attributes)
119119

120-
# Apply contig query.
121-
df = df.query(f"contig == '{contig}'")
120+
# Apply contig filter using safe boolean indexing.
121+
df = df.loc[df["contig"] == contig]
122122
return df
123123

124124
def _prep_gff_attributes(
@@ -162,9 +162,9 @@ def genome_features(
162162
contig=r.contig, attributes=attributes_normed
163163
)
164164
if r.end is not None:
165-
df_part = df_part.query(f"start <= {r.end}")
165+
df_part = df_part.loc[df_part["start"] <= r.end]
166166
if r.start is not None:
167-
df_part = df_part.query(f"end >= {r.start}")
167+
df_part = df_part.loc[df_part["end"] >= r.start]
168168
parts.append(df_part)
169169
df = pd.concat(parts, axis=0)
170170
return df.sort_values(["contig", "start"]).reset_index(drop=True).copy()
@@ -192,8 +192,8 @@ def genome_feature_children(
192192
df_gf["Parent"] = df_gf["Parent"].str.split(",")
193193
df_gf = df_gf.explode(column="Parent", ignore_index=True)
194194

195-
# Query to find children of the requested parent.
196-
df_children = df_gf.query(f"Parent == '{parent}'")
195+
# Filter to find children of the requested parent using safe indexing.
196+
df_children = df_gf.loc[df_gf["Parent"] == parent]
197197

198198
return df_children.copy()
199199

@@ -670,7 +670,9 @@ def plot_genes(
670670
def _plot_genes_setup_data(self, *, region):
671671
attributes = [a for a in self._gff_default_attributes if a != "Parent"]
672672
df_genome_features = self.genome_features(region=region, attributes=attributes)
673-
data = df_genome_features.query(f"type == '{self._gff_gene_type}'").copy()
673+
data = df_genome_features.loc[
674+
df_genome_features["type"] == self._gff_gene_type
675+
].copy()
674676
tooltips = [(a.capitalize(), f"@{a}") for a in attributes]
675677
tooltips += [("Location", "@contig:@start{,}-@end{,}")]
676678
return data, tooltips

malariagen_data/anoph/hap_data.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
import zarr # type: ignore
77
from numpydoc_decorator import doc # type: ignore
88

9+
from .safe_query import validate_query
10+
911
from ..util import (
1012
DIM_ALLELE,
1113
DIM_PLOIDY,
@@ -418,7 +420,8 @@ def haplotypes(
418420
df_samples.set_index("sample_id").loc[phased_samples].reset_index()
419421
)
420422

421-
# Apply the query.
423+
# Validate the query to prevent arbitrary code execution (GH-1292).
424+
validate_query(sample_query_prepped)
422425
sample_query_options = sample_query_options or {}
423426
loc_samples = df_samples_phased.eval(
424427
sample_query_prepped, **sample_query_options

malariagen_data/anoph/hapclust.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
from ..util import CacheMiss, _check_types, _pdist_abs_hamming, _pandas_apply
1010
from ..plotly_dendrogram import _plot_dendrogram, concat_clustering_subplots
11+
from .safe_query import validate_query
1112
from . import (
1213
base_params,
1314
plotly_params,
@@ -623,6 +624,7 @@ def transcript_haplotypes(
623624
"""
624625

625626
# Get SNP genotype allele counts for the transcript, applying snp_query
627+
validate_query(snp_query)
626628
df_eff = (
627629
self.snp_effects(
628630
transcript=transcript,

malariagen_data/anoph/karyotype.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ def load_inversion_tags(self, inversion: inversion_param) -> pd.DataFrame:
6262
else:
6363
with importlib.resources.path(resources, self._inversion_tag_path) as path:
6464
df_tag_snps = pd.read_csv(path, sep=",")
65-
return df_tag_snps.query(f"inversion == '{inversion}'").reset_index()
65+
return df_tag_snps.loc[df_tag_snps["inversion"] == inversion].reset_index()
6666

6767
@_check_types
6868
@doc(
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
"""Safe query validation for pandas eval/query expressions.
2+
3+
This module provides AST-based validation of query strings to prevent
4+
arbitrary code execution via pandas DataFrame.eval() and DataFrame.query().
5+
6+
Only a restricted subset of Python expressions is allowed:
7+
- Boolean operators: and, or, not
8+
- Comparison operators: ==, !=, <, <=, >, >=, in, not in, is
9+
- Arithmetic operators: +, -, *, /, //, %, **
10+
- Unary operators: +, -, ~, not
11+
- Constants: strings, numbers, booleans, None
12+
- Names: must match an allowlist of known column names (if provided)
13+
- Parenthesized expressions
14+
15+
Forbidden constructs include:
16+
- Function calls (e.g., __import__('os'))
17+
- Attribute access (e.g., os.system)
18+
- Subscript/indexing (e.g., x[0])
19+
- Comprehensions, lambdas, f-strings, starred expressions
20+
- Any identifier containing double underscores (__)
21+
"""
22+
23+
import ast
24+
import re
25+
from typing import Optional, Set
26+
27+
# Pattern matching pandas @variable references in query strings.
28+
# These are not valid Python but are a pandas feature for referencing
29+
# local/global variables via the `local_dict` or `global_dict` kwargs.
30+
_AT_VAR_PATTERN = re.compile(r"@([A-Za-z_][A-Za-z0-9_]*)")
31+
32+
33+
# AST node types that are safe in query expressions.
34+
_SAFE_NODE_TYPES = (
35+
ast.Expression,
36+
ast.BoolOp,
37+
ast.BinOp,
38+
ast.UnaryOp,
39+
ast.Compare,
40+
ast.And,
41+
ast.Or,
42+
ast.Not,
43+
ast.Add,
44+
ast.Sub,
45+
ast.Mult,
46+
ast.Div,
47+
ast.FloorDiv,
48+
ast.Mod,
49+
ast.Pow,
50+
ast.USub,
51+
ast.UAdd,
52+
ast.Invert,
53+
ast.Eq,
54+
ast.NotEq,
55+
ast.Lt,
56+
ast.LtE,
57+
ast.Gt,
58+
ast.GtE,
59+
ast.In,
60+
ast.NotIn,
61+
ast.Is,
62+
ast.IsNot,
63+
ast.Constant,
64+
ast.Name,
65+
ast.Load,
66+
ast.Tuple,
67+
ast.List,
68+
)
69+
70+
71+
class UnsafeQueryError(ValueError):
72+
"""Raised when a query string contains unsafe constructs."""
73+
74+
pass
75+
76+
77+
def _validate_node(node: ast.AST, allowed_names: Optional[Set[str]] = None) -> None:
78+
"""Recursively validate that an AST node contains only safe constructs.
79+
80+
Parameters
81+
----------
82+
node : ast.AST
83+
The AST node to validate.
84+
allowed_names : set of str, optional
85+
If provided, restrict identifier names to this set.
86+
87+
Raises
88+
------
89+
UnsafeQueryError
90+
If the node or any of its children contain unsafe constructs.
91+
"""
92+
if not isinstance(node, _SAFE_NODE_TYPES):
93+
raise UnsafeQueryError(
94+
f"Unsafe expression: {type(node).__name__} nodes are not allowed "
95+
f"in query strings. Only comparisons, boolean logic, and constants "
96+
f"are permitted."
97+
)
98+
99+
if isinstance(node, ast.Name):
100+
name = node.id
101+
# Block dunder identifiers.
102+
if "__" in name:
103+
raise UnsafeQueryError(
104+
f"Unsafe expression: identifier '{name}' contains double "
105+
f"underscores and is not allowed in query strings."
106+
)
107+
# Check against allowlist if provided.
108+
if allowed_names is not None and name not in allowed_names:
109+
# Allow common boolean literals that pandas recognizes.
110+
if name not in {"True", "False", "None"}:
111+
raise UnsafeQueryError(
112+
f"Unknown column name '{name}' in query string. "
113+
f"Allowed column names: {sorted(allowed_names)}"
114+
)
115+
116+
# Recurse into child nodes.
117+
for child in ast.iter_child_nodes(node):
118+
_validate_node(child, allowed_names)
119+
120+
121+
def validate_query(query: str, allowed_names: Optional[Set[str]] = None) -> None:
122+
"""Validate that a query string is safe for use with pandas eval/query.
123+
124+
Parameters
125+
----------
126+
query : str
127+
The query string to validate.
128+
allowed_names : set of str, optional
129+
If provided, restrict identifier names to this set of known column
130+
names. If None, any identifier (except those containing ``__``) is
131+
allowed.
132+
133+
Raises
134+
------
135+
UnsafeQueryError
136+
If the query contains unsafe constructs such as function calls,
137+
attribute access, or dunder identifiers.
138+
"""
139+
if not isinstance(query, str):
140+
raise UnsafeQueryError(f"Query must be a string, got {type(query).__name__}.")
141+
142+
query = query.strip()
143+
if not query:
144+
raise UnsafeQueryError("Query string must not be empty.")
145+
146+
# Replace pandas @variable references with plain identifiers so the
147+
# expression can be parsed as valid Python. The replaced names are
148+
# prefixed with ``_at_`` to avoid collisions with real column names
149+
# while remaining dunder-free.
150+
query_for_parse = _AT_VAR_PATTERN.sub(r"_at_\1", query)
151+
152+
try:
153+
tree = ast.parse(query_for_parse, mode="eval")
154+
except SyntaxError as e:
155+
raise UnsafeQueryError(f"Query string is not a valid expression: {e}") from e
156+
157+
_validate_node(tree, allowed_names)

malariagen_data/anoph/sample_metadata.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import plotly.express as px # type: ignore
2525
from numpydoc_decorator import doc # type: ignore
2626

27+
from .safe_query import validate_query
28+
2729
from ..util import _check_types
2830
from . import base_params, map_params, plotly_params
2931
from .base import AnophelesBase
@@ -808,10 +810,9 @@ def sample_metadata(
808810
# zero-result queries and provide a helpful warning.
809811
df_before_query = df_samples
810812

811-
# Use the python engine in order to support extension array dtypes, e.g. Float64, Int64, boolean.
812-
df_samples = df_samples.query(
813-
prepared_sample_query, **sample_query_options, engine="python"
814-
)
813+
# Validate the query to prevent arbitrary code execution (GH-1292).
814+
validate_query(prepared_sample_query)
815+
df_samples = df_samples.query(prepared_sample_query, **sample_query_options)
815816
df_samples = df_samples.reset_index(drop=True)
816817

817818
# Warn if query returned zero results on a non-empty dataset.
@@ -1197,12 +1198,13 @@ def _prep_sample_selection_cache_params(
11971198
# Default the sample_query_options to an empty dict.
11981199
sample_query_options = sample_query_options or {}
11991200

1200-
# Use the python engine in order to support extension array dtypes, e.g. Float64, Int64, boolean.
1201+
# Validate the query to prevent arbitrary code execution (GH-1292).
12011202
# Get the Pandas Series as a NumPy array of Boolean values.
12021203
# Note: if `prepared_sample_query` is an internal query, this will select all samples,
12031204
# since `sample_metadata` should have already applied the internal query.
1205+
validate_query(prepared_sample_query)
12041206
loc_samples = df_samples.eval(
1205-
prepared_sample_query, **sample_query_options, engine="python"
1207+
prepared_sample_query, **sample_query_options
12061208
).values
12071209

12081210
# Convert the sample indices to a list.
@@ -1368,6 +1370,7 @@ def _setup_sample_symbol(
13681370
)
13691371
data["symbol"] = ""
13701372
for key, value in symbol.items():
1373+
validate_query(value)
13711374
data.loc[data.query(value).index, "symbol"] = key
13721375
symbol_prepped = "symbol"
13731376

@@ -1421,6 +1424,7 @@ def _setup_sample_colors_plotly(
14211424
)
14221425
data["color"] = ""
14231426
for key, value in color.items():
1427+
validate_query(value)
14241428
data.loc[data.query(value).index, "color"] = key
14251429
color_prepped = "color"
14261430

@@ -1654,6 +1658,7 @@ def cohorts(
16541658
self._cache_cohorts[cache_key] = df_cohorts
16551659

16561660
if query is not None:
1661+
validate_query(query)
16571662
df_cohorts = df_cohorts.query(query)
16581663
df_cohorts = df_cohorts.reset_index(drop=True)
16591664

@@ -1872,6 +1877,7 @@ def _locate_cohorts(*, cohorts, data, min_cohort_size):
18721877

18731878
for coh, query in cohorts.items():
18741879
try:
1880+
validate_query(query)
18751881
loc_coh = data.eval(query).values
18761882
except (KeyError, NameError, SyntaxError, TypeError, AttributeError) as e:
18771883
raise ValueError(

0 commit comments

Comments
 (0)