Conversation
Stop reflecting untrusted request paths into the app HTML; default invalid/out-of-root paths to / and only set initial path when it’s an existing directory. Also escape terminal prompt and inline image HTML to avoid DOM/attribute injection. Add regression tests for the reported payload. Thanks to Keyur Maheta for the responsible disclosure.
There was a problem hiding this comment.
Pull request overview
This PR fixes a reflected XSS vulnerability where untrusted request paths could be injected into the HTML response. The fix ensures that only validated, existing directory paths are used as the initial path, and all user-controlled values are properly escaped before rendering in HTML or JavaScript contexts.
Key changes:
- Paths that don't exist or are outside the root now default to
/instead of being reflected - HTML escaping applied to initial paths, terminal prompts, and image display
- Client-side path validation to ensure paths start with
/
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| main.go | Adds path validation to only pass existing directories to serveMainIndex; adds HTML escaping to processHTMLTemplate, image HTML generation, and no-JS directory listings; adds redirect response for directories with index.html |
| index.html | Moves initial path from direct template substitution to data attribute; adds escapeHTML to terminal prompt rendering; adds client-side path validation; adds redirect handling |
| handlers_test.go | Adds regression tests for XSS payload rejection and valid directory path handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| size = fmt.Sprintf(" (%d bytes)", info.Size()) | ||
| } | ||
| _, _ = fmt.Fprintf(w, "<a href=\"%s\">%s</a>%s<br>\n", filePath, file.Name(), size) | ||
| _, _ = fmt.Fprintf(w, "<a href=\"%s\">%s</a>%s<br>\n", urlEscapeVirtual(filePath), template.HTMLEscapeString(file.Name()), size) |
There was a problem hiding this comment.
The urlEscapeVirtual function does not escape HTML-sensitive characters like quotes, angle brackets, or apostrophes. When used in href attributes without additional HTML escaping, this could allow attribute injection. For defense in depth, consider applying template.HTMLEscapeString to URL values used in HTML attributes, or extend urlQueryEscape to handle quotes and angle brackets. For example, a filename containing a double quote could break out of the href attribute.
| if virtualPath != "/" { | ||
| parentPath := path.Dir(virtualPath) | ||
| _, _ = fmt.Fprintf(w, "<a href=\"%s?nojs=1\">[Parent Directory]</a><br>\n", parentPath) | ||
| _, _ = fmt.Fprintf(w, "<a href=\"%s?nojs=1\">[Parent Directory]</a><br>\n", urlEscapeVirtual(parentPath)) |
There was a problem hiding this comment.
The urlEscapeVirtual function does not escape HTML-sensitive characters like quotes, angle brackets, or apostrophes. When used in href attributes without additional HTML escaping, this could allow attribute injection. For defense in depth, consider applying template.HTMLEscapeString to URL values used in HTML attributes, or extend urlQueryEscape to handle quotes and angle brackets. For example, a filename containing a double quote could break out of the href attribute.
| for _, dir := range dirs { | ||
| dirPath := path.Join(virtualPath, dir.Name()) | ||
| _, _ = fmt.Fprintf(w, "<a href=\"%s?nojs=1\">%s/</a><br>\n", dirPath, dir.Name()) | ||
| _, _ = fmt.Fprintf(w, "<a href=\"%s?nojs=1\">%s/</a><br>\n", urlEscapeVirtual(dirPath), template.HTMLEscapeString(dir.Name())) |
There was a problem hiding this comment.
The urlEscapeVirtual function does not escape HTML-sensitive characters like quotes, angle brackets, or apostrophes. When used in href attributes without additional HTML escaping, this could allow attribute injection. For defense in depth, consider applying template.HTMLEscapeString to URL values used in HTML attributes, or extend urlQueryEscape to handle quotes and angle brackets. For example, a filename containing a double quote could break out of the href attribute.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…tribute injection (#51) * Initial plan * Use stdlib url.PathEscape and html.EscapeString for defense in depth Co-authored-by: puria <10379+puria@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: puria <10379+puria@users.noreply.github.com>
Stop reflecting untrusted request paths into the app HTML; default invalid/out-of-root paths to / and only set
initial path when it’s an existing directory. Also escape terminal prompt and inline image HTML to avoid DOM/attribute
injection. Add regression tests for the reported payload.
Thanks to Keyur Maheta for the responsible disclosure.