Skip to content

fix: harden getSdkAuthorizationData against injection and parsing bugs#1459

Open
aritro2002 wants to merge 1 commit intomainfrom
fix-sdk-authorization
Open

fix: harden getSdkAuthorizationData against injection and parsing bugs#1459
aritro2002 wants to merge 1 commit intomainfrom
fix-sdk-authorization

Conversation

@aritro2002
Copy link
Copy Markdown
Contributor

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

Description

Hardened the getSdkAuthorizationData function in Utils.res against injection and XSS attacks. The function decodes a base64-encoded SDK authorization token and extracts key-value pairs (publishableKey, clientSecret, customerId, profileId) that flow into downstream consumers. Previously, no input validation was applied to these user-controlled values, and several parsing bugs existed.

Vulnerabilities fixed:

  1. Unhandled atob exception -- Window.atob throws DOMException on invalid base64 input. Added try/catch around the entire function body, returning an empty result (all None) on failure.

  2. Value truncation on = characters -- String.split("=") -> Array.get(1) discarded everything after the first =, silently corrupting values containing = (e.g., base64 padding). Replaced with indexOf("=") + sliceToEnd to capture the full value.

  3. False-positive key matching -- String.includes(keyName) used substring matching, so a key like "key" would match "publishable_key". Replaced with String.startsWith(keyName ++ "=") for exact prefix matching.

  4. No input validation -- Extracted values were passed directly to downstream code with no sanitization. Added per-field allowlist regex validation using anchored (^...$) patterns:

    • publishableKey: ^pk_(prd|snd|dev)_[a-zA-Z0-9]+$
    • clientSecret: ^[A-Za-z0-9_-]+_secret_[A-Za-z0-9]+$
    • customerId: ^[a-zA-Z0-9_-]+$
    • profileId: ^[a-zA-Z0-9_-]+$

    Fields that fail validation are returned as None, preventing malicious payloads from propagating.

Design decisions:

  • Regexes are intentionally format-agnostic (not tied to specific backend ID formats) since those formats may change.
  • CSP is deployed via <meta> tag as defense-in-depth but is NOT relied upon as the sole XSS mitigation.
  • JSON.stringify was evaluated and rejected as an XSS mitigation since it does not escape <, >, (, ).

Changes Summary

Modified Files:

  • src/Utilities/Utils.res -- getSdkAuthorizationData function (lines 1910-1956): added try/catch for atob, fixed value extraction, fixed key matching, added per-field regex validation.

How did you test it?

Normal Flow:
image

Checklist

  • I ran npm run re:build
  • I reviewed submitted code
  • I added unit tests for my changes where possible

@semanticdiff-com
Copy link
Copy Markdown

Review changes with  SemanticDiff

@github-actions
Copy link
Copy Markdown
Contributor

🚫 Missing Linked Issue

Hi 👋 This pull request does not appear to be linked to any open issue yet.

Linking your PR to an issue helps keep the project tidy and ensures the issue is closed automatically.

✔️ How to fix this

  • Add a keyword like Fixes #123 or Closes #456 to your PR description or a commit message.
  • Or link it manually using the "Linked issues" panel in the PR sidebar.

Tip: You can link multiple issues.
🚫 Note: If only one issue is linked, it must be open for this check to pass.

Once linked, this check will pass automatically on your next push or when you re-run the workflow.

Thanks for helping maintainers! 🙌

@aritro2002 aritro2002 added Ready for Review PR with label Ready for Review should only be reviewed. DO NOT MERGE Use this label if you want your PR to restrict from merging and removed Ready for Review PR with label Ready for Review should only be reviewed. labels Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DO NOT MERGE Use this label if you want your PR to restrict from merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant