rusackas commented on code in PR #43398:
URL: https://github.com/apache/superset/pull/43398#discussion_r3832078218
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx:
##########
@@ -78,6 +78,35 @@
export const SEARCH_THRESHOLD = 10;
+/**
+ * Escape a single CSV cell value.
+ *
+ * Mirrors the server-side chokepoint (superset/utils/csv.py escape_value):
+ * values starting with a spreadsheet formula prefix (=, +, -, @, |, %, or a
+ * leading tab/carriage return, optionally behind leading whitespace) are
+ * neutralized with a leading single quote so exported cells cannot execute
+ * as formulas when opened in Excel/LibreOffice/Google Sheets. Plain negative
+ * numbers are left untouched. RFC-4180 quoting is applied afterwards.
+ */
+export const escapeCsvValue = (v: unknown): string => {
+ if (v === null || v === undefined) return '';
+ let s = String(v);
+ if (s.length > 0) {
+ const stripped = s.replace(/^\s+/, '');
+ const startsLikeFormula =
+ s[0] === '\t' ||
+ s[0] === '\r' ||
+ (stripped.length > 0 && '-@+|=%'.includes(stripped[0]));
+ const isNegativeNumber = s.length > 1 && /^-[0-9.]+$/.test(s);
+ if (startsLikeFormula && !isNegativeNumber) {
+ // Escape pipe to be extra safe (DDE payloads), then prefix with a
+ // single quote to prevent formula evaluation.
+ s = `'${s.replace(/\|/g, '\\|')}`;
Review Comment:
Good catch, fixed in 3e6b2b323a.
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx:
##########
@@ -78,6 +78,35 @@ import { useExploreDataExport } from
'./useExploreDataExport';
export const SEARCH_THRESHOLD = 10;
+/**
+ * Escape a single CSV cell value.
+ *
+ * Mirrors the server-side chokepoint (superset/utils/csv.py escape_value):
+ * values starting with a spreadsheet formula prefix (=, +, -, @, |, %, or a
+ * leading tab/carriage return, optionally behind leading whitespace) are
+ * neutralized with a leading single quote so exported cells cannot execute
+ * as formulas when opened in Excel/LibreOffice/Google Sheets. Plain negative
+ * numbers are left untouched. RFC-4180 quoting is applied afterwards.
+ */
+export const escapeCsvValue = (v: unknown): string => {
+ if (v === null || v === undefined) return '';
+ let s = String(v);
+ if (s.length > 0) {
+ const stripped = s.replace(/^\s+/, '');
+ const startsLikeFormula =
+ s[0] === '\t' ||
+ s[0] === '\r' ||
+ (stripped.length > 0 && '-@+|=%'.includes(stripped[0]));
+ const isNegativeNumber = s.length > 1 && /^-[0-9.]+$/.test(s);
+ if (startsLikeFormula && !isNegativeNumber) {
+ // Escape pipe to be extra safe (DDE payloads), then prefix with a
+ // single quote to prevent formula evaluation.
+ s = `'${s.replace(/\|/g, '\\|')}`;
+ }
+ }
+ return /[",\n]/.test(s) ? `"${s.replace(/"/g, '""')}"` : s;
Review Comment:
Good catch, fixed in the same commit (3e6b2b323a).
##########
superset-frontend/src/pages/SqlLab/LocationContext.tsx:
##########
@@ -41,12 +41,18 @@ export const LocationProvider: FC<{ children?: ReactNode }>
= ({
const queryParams = new URLSearchParams(location.search);
const permalink = location.pathname.match(/\/p\/\w+/)?.[0].slice(3);
if (queryParams.size > 0 || permalink) {
- const autorun = queryParams.get('autorun') === 'true';
+ // SECURITY: never honor `autorun` from the URL querystring. A crafted
+ // cross-site GET link (?dbid=..&sql=..&autorun=true) is a top-level
+ // navigation, so SameSite=Lax cookies are sent and the attacker-chosen
+ // SQL would execute in the victim's session the moment SQL Lab mounts.
+ // Only in-app navigations that pass `location.state` (handled above)
+ // may request autorun; deep links prefill the editor and wait for the
+ // user to press Run.
const isDataset = queryParams.get('isDataset') === 'true';
const queryParamsState = {
requestedQuery: {
...Object.fromEntries(queryParams),
- autorun,
+ autorun: false,
Review Comment:
Traced this through - requested_query only populates from a POST to
superset.views.sqllab, which isn't in WTF_CSRF_EXEMPT_LIST, so a cross-site
attacker can't forge that request. Leaving this one as-is; the GET-link path
this PR fixes is the actual forgeable vector.
--
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]
