Potential fix for code scanning alert no. 56: Client-side URL redirect#26
Potential fix for code scanning alert no. 56: Client-side URL redirect#26
Conversation
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| var b, c; | ||
| b = location.href.search(/:/)==2 ? 14 : 7; | ||
| c = location.href.lastIndexOf("\\")+1; | ||
| a = "file:///" + location.href.substring(b, c) + a; |
Check warning
Code scanning / CodeQL
Client-side URL redirect Medium documentation
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the client-side URL redirect issue, we should sanitize and strictly validate all parts that contribute to the redirect URL. In particular, we should avoid using unchecked data from location.href to construct file paths, and instead use a safer mechanism. Since the function intends to redirect to a locally available help file referenced by a filename, it should select from a hardcoded list of allowed filenames or simply use the provided parameter a as a relative filename, not concatenated with user-controlled path data. If directory navigation is needed, define an allowed set of directories (preferably hardcoded), and ignore any values derived from the current location. This prevents attackers from manipulating the redirect path via location.
Therefore, the best way to fix the problem is to keep the allowed filenames in a white-list array, verify a is in that array, and construct the redirect path only from trusted base directory + safe filename, avoiding the use of location.href for redirect destination construction.
Edits needed: In src/doc/files/js/chmRelative.js, update the implementation of relativelnk such that:
- A hardcoded base directory is used (for local help files, e.g.,
file:///C:/Program Files/HelpFiles/). - The function checks that
ais in a whitelist of allowed filenames. - Only construct the redirect URL from trusted parts, completely avoiding use of
location.hreffor redirect destination.
The whitelist should be declared in the same file.
| @@ -1,12 +1,20 @@ | ||
| // Whitelist of allowed help file names (add entries as needed) | ||
| const allowedHelpFiles = [ | ||
| "index.html", | ||
| "getting_started.html", | ||
| "faq.html", | ||
| "installation.html", | ||
| "advanced_topics.html" | ||
| ]; | ||
|
|
||
| function relativelnk(a){ | ||
| // Simple path validation: allow only relative filenames (no slashes, no colon, no protocol) | ||
| if (typeof a !== 'string' || a.match(/^[a-zA-Z0-9_.-]+$/) === null) { | ||
| console.error('Invalid path for redirect'); | ||
| // Only allow a to be from the whitelist of help files | ||
| if (typeof a !== 'string' || !allowedHelpFiles.includes(a)) { | ||
| console.error('Invalid or unauthorized help file requested; aborting redirect.'); | ||
| return; | ||
| } | ||
| var b, c; | ||
| b = location.href.search(/:/)==2 ? 14 : 7; | ||
| c = location.href.lastIndexOf("\\")+1; | ||
| a = "file:///" + location.href.substring(b, c) + a; | ||
| location.href = a; | ||
| // Use a fixed trusted help files directory | ||
| var baseHelpPath = "file:///C:/Program Files/HelpFiles/"; | ||
| var targetURL = baseHelpPath + a; | ||
| location.href = targetURL; | ||
| }; |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
HelpNDocTools/src/doc/files/js/chmRelative.js
Lines 2 to 6 in be4ace0
Path validation rejects legitimate nested relative links
The new regex (^[a-zA-Z0-9_.-]+$) only allows bare filenames with letters, digits, dot, underscore, or hyphen and returns early for anything else. Any link that includes a directory separator (e.g. images/foo.htm, ../index.htm) or even a space—both common in HelpNDoc output—will now be treated as invalid and the navigation will silently fail. This fixes the redirect alert but introduces a broad functional regression. Consider validating that the path stays relative (no protocol, no drive letters) rather than forbidding slashes entirely.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Potential fix for https://github.com/paule32/HelpNDocTools/security/code-scanning/56
To mitigate client-side URL redirect vulnerabilities, validate or constrain the value of
aso only safe, expected URLs/paths may be used for redirection. Since this code is constructing afile:///URL using part of the current location and the valuea(likely meant as a relative link or path fragment), the most effective fix is to ensure thatacannot contain characters or patterns that break out into absolute URLs or inject malicious content.A robust fix involves:
a.aso it does not contain dangerous characters ("://", "", ":", etc.), or start with "http", "file", "//", etc.aagainst a whitelist of permitted filenames, if possible.The code to change is in
src/doc/files/js/chmRelative.jsin the definition ofrelativelnk. You may need to add a helper function to perform path validation or sanitization.No external dependencies are required for basic string validation/sanitization.
Suggested fixes powered by Copilot Autofix. Review carefully before merging.