codeant-ai-for-open-source[bot] commented on code in PR #41132:
URL: https://github.com/apache/superset/pull/41132#discussion_r3425015963
##########
superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.ts:
##########
@@ -187,9 +186,11 @@ function CountryMap(element: HTMLElement, props:
CountryMapProps) {
region => region.country_id === d.properties.ISO,
);
- hoverPopup.style('display', 'block').html(
- `<div><strong>${getNameOfRegion(d)}</strong><br>${result.length > 0 ?
formatter(result[0].metric) : ''}</div>`,
- );
+ hoverPopup
+ .style('display', 'block')
+ .html(
+ `<div><strong>${getNameOfRegion(d)}</strong><br>${result.length > 0 ?
formatter(result[0].metric) : ''}</div>`,
+ );
Review Comment:
**Suggestion:** This tooltip now injects `getNameOfRegion(d)` and formatted
metric output directly into `.html(...)` without escaping. If either value
contains HTML (for example from dataset-derived formatted values), it will be
executed in the browser and creates an XSS vector. Escape both interpolated
values (or render with text nodes) before calling `.html(...)`. [security]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Country Map tooltips can execute injected HTML/JavaScript.
- ❌ Dashboards embedding Country Map expose XSS attack surface.
- ⚠️ Compromised charts can exfiltrate data or user session tokens.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Superset registers the Country Map visualization via `MainPreset` at
`superset-frontend/src/visualizations/presets/MainPreset.ts:20-23,100-116`,
where `new
CountryMapChartPlugin().configure({ key: VizType.CountryMap })` is added to
the `plugins`
array, making this viz type available in the Explore UI.
2. In the Country Map control panel
(`superset-frontend/plugins/legacy-plugin-chart-country-map/src/controlPanel.ts:58-69`),
the `number_format` control is defined as a `SelectControl` with `freeForm:
true`, and
datasource-level `currencyFormats` / `columnFormats` are passed through from
the backend
without escaping in `transformProps` (`transformProps.ts:33-39`). A
malicious or buggy
configuration can thus cause `getValueFormatter` (imported from
`@superset-ui/core` at
`transformProps.ts:19` and invoked at `transformProps.ts:40-49`) to return a
`formatter`
function that outputs strings containing arbitrary HTML markup derived from
configuration
or data.
3. When a Country Map chart is rendered, `transformProps` returns this
`formatter` in the
props (`transformProps.ts:52-61`). The plugin's React wrapper in
`ReactCountryMap.tsx:26-44` (`ReactComponent` from `reactify`) forwards all
props to the
imperative `CountryMap` render function (`CountryMap.ts:71-81`), so
`CountryMap` receives
the `formatter` function directly as `props.formatter`.
4. At runtime, when a user hovers a region on the Country Map, the
`mouseenter` handler in
`CountryMap.ts:177-195` executes. It looks up the matching data row (`result
=
data.filter(...)` at `CountryMap.ts:185-187`) and then calls
`hoverPopup.html(\`<div><strong>${getNameOfRegion(d)}</strong><br>${result.length
> 0 ?
formatter(result[0].metric) : ''}</div>\`)` at `CountryMap.ts:189-193`.
Because
`.html(...)` is used and there is no escaping on either `getNameOfRegion(d)`
or
`formatter(result[0].metric)`, any HTML returned by `formatter` (or any HTML
present in a
future change where region names or metrics are allowed to contain markup)
will be
injected directly into the DOM and executed by the browser, creating an XSS
vector in the
Country Map tooltip.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=74a1c45081f34a5d9e73040140f8a395&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=74a1c45081f34a5d9e73040140f8a395&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.ts
**Line:** 189:193
**Comment:**
*Security: This tooltip now injects `getNameOfRegion(d)` and formatted
metric output directly into `.html(...)` without escaping. If either value
contains HTML (for example from dataset-derived formatted values), it will be
executed in the browser and creates an XSS vector. Escape both interpolated
values (or render with text nodes) before calling `.html(...)`.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41132&comment_hash=d2b08f89aa5f1bbdb46adf66f90e8e7e9566a8c1e0e76d845f330afa133d27d0&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41132&comment_hash=d2b08f89aa5f1bbdb46adf66f90e8e7e9566a8c1e0e76d845f330afa133d27d0&reaction=dislike'>👎</a>
--
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]
