Potential fix for code scanning alert no. 21: DOM text reinterpreted as HTML

[thorsten]

Potential fix for https://github.com/thorsten/phpMyFAQ/security/code-scanning/21

In general, the problem is resolved by validating or sanitizing data read from the DOM before using it in security-sensitive contexts such as constructing URLs, HTML, or selectors. Here, nextStep.value should be constrained to the expected set of step identifiers (for example, positive integers) before interpolating it into the query string used with window.location.replace.

The best minimal fix that preserves existing functionality is to parse nextStep.value as an integer and ensure it is a valid, positive number before using it. If parsing fails or yields an invalid number, we can either abort navigation or fall back to a safe default (e.g. step 1). This ensures that characters which could significantly alter the URL structure (like : or ?) are not propagated, because only digits are accepted. No new imports are needed.

Concretely, in phpmyfaq/assets/src/configuration/update.ts, within handleUpdateNextStepButton, replace the direct use of `?step=${nextStep.value}` with logic that:

1. Parses nextStep.value using parseInt.
2. Checks Number.isNaN and numeric bounds.
3. Constructs the URL using the sanitized numeric step.

Only the body of the click event listener needs changing; the rest of the file remains as is.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

Summary by CodeRabbit

• Bug Fixes
  • Improved step navigation validation to properly reject invalid or zero step values. Only valid numeric steps are now processed during navigation, enhancing reliability and preventing navigation errors.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Input validation has been added to the next-step navigation handler in the configuration update module. The handler now parses the step value as an integer, validates it is greater than zero, and only navigates when valid—replacing the previous approach that used the raw string value directly.

Changes

Cohort / File(s)	Summary
Input Validation phpmyfaq/assets/src/configuration/update.ts	Added integer parsing and validation for next-step value; ignores invalid or values less than 1 before location update

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A hop through the code, a check here, a rule,
No bogus steps now—we're nobody's fool!
Parse, validate, navigate with care,
Integers only, pristine and fair! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly relates to the main change: input validation for DOM values before URL construction addresses the code scanning alert about DOM text reinterpreted as HTML.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Docstrings were successfully generated.

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch alert-autofix-21

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

phpmyfaq/assets/src/configuration/update.ts (1)

161-161: Critical XSS vulnerability: Unsafe use of innerHTML with API response.

Using innerHTML with unsanitized API response data (result.error) creates an XSS vulnerability. If the error message contains malicious HTML or JavaScript, it will execute in the user's browser.

🔎 Proposed fix

Replace innerHTML with innerText (consistent with lines 55, 83, 149, 169):

-errorMessage.innerHTML = result.error;
+errorMessage.innerText = result.error;

If HTML formatting in error messages is intentionally required, you must sanitize result.error using a trusted sanitization library (e.g., DOMPurify) before assigning to innerHTML.

🧹 Nitpick comments (1)

phpmyfaq/assets/src/configuration/update.ts (1)

23-27: Security fix correctly implemented.

The input validation effectively addresses the code-scanning alert by:

• Parsing the DOM value as an integer with explicit radix
• Rejecting NaN, zero, and negative values
• Using only the sanitized numeric value in the URL

This prevents injection of characters that could alter URL structure.

Optional: Consider adding an upper bound check

For additional robustness, you could add an upper bound to prevent unreasonably large step values:

 const stepValue = parseInt(nextStep.value, 10);
-if (Number.isNaN(stepValue) || stepValue < 1) {
+if (Number.isNaN(stepValue) || stepValue < 1 || stepValue > 100) {
   return;
 }

Adjust the upper bound (100 is illustrative) based on your actual number of update steps.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0e80970 and 7d4c497.

📒 Files selected for processing (1)

• phpmyfaq/assets/src/configuration/update.ts

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript coding standards for TypeScript code

Files:

• phpmyfaq/assets/src/configuration/update.ts

**/*.{php,ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{php,ts,tsx,js,jsx}: Use semicolons at the end of each statement
Use single quotes for strings
Follow best practices for localization, such as using placeholders for dynamic content and avoiding hard-coded strings

Files:

• phpmyfaq/assets/src/configuration/update.ts

**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use arrow functions for callbacks

Files:

• phpmyfaq/assets/src/configuration/update.ts

[coderabbitai]

Caution

Docstrings generation - FAILED

No docstrings were generated.