Skip to content

fix(frontend): XSS escaping gaps, confirm() to confirmDialog, alert() to toast #1065

Closed

Description

@cristim

Problem

FOLD-1046 findings from code review report 11, added to PR #1046.

11-M1 -- plans.ts: plan numeric fields not validated before parseInt

savePlan reads target_coverage, notification_days_before, custom_step_percent, custom_interval_days via parseInt with no Number.isInteger(Number(raw)) check. Malformed input silently produces NaN or a truncated integer.

11-M2 -- users/filters.ts: unescaped group.id in option value attribute

updateGroupFilterDropdown injects group.id raw into innerHTML (value=group.id). Violates feedback_innerhtml_xss policy.

11-M3 -- groups/groupModals.ts: max_amount not validated before parseFloat

collectPermissions calls parseFloat(maxAmount) without Number.isFinite + >= 0 guard.

11-L1 -- plans.ts: unescaped data-id interpolations in innerHTML

plan.id and purchase.id injected raw into data-id attributes in innerHTML templates.

11-L2 -- plans.ts: blocking confirm() dialogs

handlePlannedPurchaseAction uses confirm() for Run and Disable. Replace with confirmDialog.

11-L3 -- several files: alert() calls

recommendations.ts (refreshRecommendations), auth.ts (updateProfile), federation.ts, modules/registrations.ts use alert(). Replace with showToast/showError.

11-N1 -- utils.ts: deepClone should default to structuredClone

JSON.parse/stringify drops undefined, converts Date, loses Set/Map.

11-N2 -- utils.ts: formatCurrency renders absent/NaN same as 0

Missing data indistinguishable from real zero.

Fix

Wrap group.id in escapeHtml, guard max_amount with Number.isFinite, validate plan numeric fields strictly, escape data-id attrs, replace confirm() and alert() with confirmDialog/showToast, default deepClone to structuredClone, distinguish absent from zero in formatCurrency.

Evidence

Report 11: 11-M1, 11-M2, 11-M3, 11-L1, 11-L2, 11-L3, 11-N1, 11-N2.

Metadata

Metadata

Assignees

No one assigned

    Labels

    effort/mDaysimpact/manyAffects most userspr-createdA PR has been opened for this issue (dedup guard for the auto-PR loop)pr-mergedThe PR for this issue has been mergedpriority/p2Backlog-worthyseverity/mediumModerate harmtriagedItem has been triagedtype/securitySecurity findingurgency/this-sprintWithin the current sprint

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions