bito-code-review[bot] commented on code in PR #43560:
URL: https://github.com/apache/superset/pull/43560#discussion_r3865461092
##########
superset/utils/excel.py:
##########
@@ -21,21 +21,33 @@
from superset.utils.core import GenericDataType
+# Leading characters that turn a cell into a formula in spreadsheet apps.
+FORMULA_PREFIXES = {"=", "+", "-", "@"}
+
+
+def _quote_formula(value: Any) -> Any:
+ """Prefix a string with a quote when it would parse as a formula."""
+ return (
+ f"'{value}"
+ if isinstance(value, str) and len(value) and value[0] in
FORMULA_PREFIXES
+ else value
+ )
+
def quote_formulas(df: pd.DataFrame) -> pd.DataFrame:
"""
Make sure to quote any formulas for security reasons.
"""
- formula_prefixes = {"=", "+", "-", "@"}
-
- for col in df.select_dtypes(include="object").columns:
- df[col] = df[col].apply(
- lambda x: (
- f"'{x}"
- if isinstance(x, str) and len(x) and x[0] in formula_prefixes
- else x
- )
- )
+ # Columns are addressed by position rather than by label: a dataframe can
+ # carry duplicate column labels (the verbose_map rename in
+ # QueryContextProcessor.get_data can collapse two columns onto the same
+ # name), and ``df[label]`` then yields a DataFrame instead of a Series.
+ # ``DataFrame.apply`` would hand whole columns to the mapper rather than
+ # individual cells, silently leaving formulas unquoted.
+ for idx in range(len(df.columns)):
+ series = df.iloc[:, idx]
+ if series.dtype == object:
+ df.isetitem(idx, series.map(_quote_formula))
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>CWE-79: Formula quoting bypassed</b></div>
<div id="fix">
In pandas 3.x, string columns have dtype `str` instead of `object`, so the
check `series.dtype == object` on line 49 always returns `False`. Formulas in
string columns are never quoted, allowing formula injection in exported Excel
files. (See also: [CWE-79](https://cwe.mitre.org/data/definitions/79.html))
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
for idx in range(len(df.columns)):
series = df.iloc[:, idx]
if pd.api.types.is_object_dtype(series.dtype) or
pd.api.types.is_string_dtype(series.dtype):
df.isetitem(idx, series.map(_quote_formula))
````
</div>
</details>
</div>
<small><i>Code Review Run #746c13</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
