AnalyticsInsights pipes the location name into dangerouslySetInnerHTML, and the insight request is never cancelled
1. The insight text is rendered as raw HTML
src/components/AnalyticsInsights.jsx:
<p
style={{ ... }}
dangerouslySetInnerHTML={{ __html: insight.description.replace(/\*\*(.*?)\*\*/g, '<strong>$1</strong>') }}
/>The intent is only to turn **bold** into <strong>. But replace runs on the string and then the whole string is handed to innerHTML, so every other character in it is interpreted as markup too.
insight.description is not a constant. src/services/aiInsightsService.js builds it by interpolation, and one of the values it interpolates is the location name:
description: `Based on the past year in ${cityName || 'this location'}, **${formatMonth(highestMonth)}** had the highest average pollution ...`cityName reaches AnalyticsInsights from Dashboard, which gets it from the location search — i.e. from whatever the geocoder returned for text the user typed. That is third-party data on a path to innerHTML, which is the definition of an XSS sink. A place name (or a doctored/cached geocoder response) containing
<img src=x onerror="...">
executes in the page. The app stores community reports, exposure logs and leaderboard identity in localStorage on the same origin.
React escapes by default and the rest of this component relies on that — {insight.title}, {insight.source}, {insight.confidence} are all safe. This one line opts out of it, and it is the only line in the file that does.
It is also fragile independently of security: insight.description.replace throws if a future insight is pushed without a description.
The repo has been here before — #497 was the double-escaping fix in CommunityHub, whose comment now reads "component never uses dangerouslySetInnerHTML". That rule is right and should hold here too.
2. The fetch is not cancelled, so an older city can overwrite a newer one
useEffect(() => {
if (lat != null && lon != null) {
setLoadingInsights(true);
generateAIInsights(lat, lon, cityName)
.then(result => { ... setInsights(result.insights); })
...
}
}, [lat, lon, cityName]);No cleanup, no generation guard. generateAIInsights goes through fetchHistoricalData, which is a year of hourly archive data — slow, and slower on a cold cache. Search Delhi, then immediately search Mumbai: two requests are in flight, and whichever resolves last wins. If Delhi's resolves second the panel shows Delhi's insights under Mumbai's heading, with loadingInsights already false so nothing signals it.
The same effect also calls setInsights / setLoadingInsights after the component unmounts, which is the React warning this pattern always produces.
Expected
**bold** is rendered by producing React elements, not by writing HTML — so everything outside the ** markers stays escaped text. No dangerouslySetInnerHTML in this component.- A non-string or missing
description renders as empty rather than throwing.
- The effect ignores a resolution that is no longer the current request (cleanup flag or a generation counter), so the panel always shows the insights for the city currently selected.
AnalyticsInsightspipes the location name intodangerouslySetInnerHTML, and the insight request is never cancelled1. The insight text is rendered as raw HTML
src/components/AnalyticsInsights.jsx:The intent is only to turn
**bold**into<strong>. Butreplaceruns on the string and then the whole string is handed toinnerHTML, so every other character in it is interpreted as markup too.insight.descriptionis not a constant.src/services/aiInsightsService.jsbuilds it by interpolation, and one of the values it interpolates is the location name:description: `Based on the past year in ${cityName || 'this location'}, **${formatMonth(highestMonth)}** had the highest average pollution ...`cityNamereachesAnalyticsInsightsfromDashboard, which gets it from the location search — i.e. from whatever the geocoder returned for text the user typed. That is third-party data on a path toinnerHTML, which is the definition of an XSS sink. A place name (or a doctored/cached geocoder response) containingexecutes in the page. The app stores community reports, exposure logs and leaderboard identity in
localStorageon the same origin.React escapes by default and the rest of this component relies on that —
{insight.title},{insight.source},{insight.confidence}are all safe. This one line opts out of it, and it is the only line in the file that does.It is also fragile independently of security:
insight.description.replacethrows if a future insight is pushed without adescription.The repo has been here before — #497 was the double-escaping fix in
CommunityHub, whose comment now reads "component never uses dangerouslySetInnerHTML". That rule is right and should hold here too.2. The fetch is not cancelled, so an older city can overwrite a newer one
No cleanup, no generation guard.
generateAIInsightsgoes throughfetchHistoricalData, which is a year of hourly archive data — slow, and slower on a cold cache. Search Delhi, then immediately search Mumbai: two requests are in flight, and whichever resolves last wins. If Delhi's resolves second the panel shows Delhi's insights under Mumbai's heading, withloadingInsightsalready false so nothing signals it.The same effect also calls
setInsights/setLoadingInsightsafter the component unmounts, which is the React warning this pattern always produces.Expected
**bold**is rendered by producing React elements, not by writing HTML — so everything outside the**markers stays escaped text. NodangerouslySetInnerHTMLin this component.descriptionrenders as empty rather than throwing.