[wasm] Html Encode incoming parameters to debug page#130960
Conversation
|
Azure Pipelines: Successfully started running 3 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
This PR updates the WASM DevServer debugging help UI to HTML-encode query-derived values before writing them into the generated HTML, reducing the risk of HTML/script injection when rendering diagnostic pages.
Changes:
- Pre-encode
urlquery parameter and the computed debugger tabs list URL, and reuse the encoded values in error/help HTML. - Update the “no matching targets” message to reuse the pre-encoded strings.
Comments suppressed due to low confidence (1)
src/mono/wasm/host/DevServer/WebAssemblyNetDebugProxyAppBuilderExtensions.cs:363
- The exception is written into the HTML response without encoding (
<pre>{ex}</pre>). Exception messages can include user-controlled values (e.g., host/URL strings) and should be HTML-encoded to avoid XSS in the debug page.
<h2>Underlying exception:</h2>
<pre>{ex}</pre>
");
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/mono/wasm/host/DevServer/WebAssemblyNetDebugProxyAppBuilderExtensions.cs:362
- The underlying exception is written into HTML without encoding (
<pre>{ex}</pre>). Exception messages/stack traces can include characters like</&(and sometimes user-derived values), which can break the page or enable HTML injection. Encode the exception text before writing it to the response.
{GetLaunchEdgeInstructions(targetApplicationUrlEncoded)}
</p>
<strong>This should launch a new browser window with debugging enabled..</p>
<h2>Underlying exception:</h2>
<pre>{ex}</pre>
|
/ba-g MCP failure is unrelated and code review has run on backports |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
src/mono/wasm/host/DevServer/WebAssemblyNetDebugProxyAppBuilderExtensions.cs:360
- The generated HTML in the error page is malformed: it opens a tag but closes with , and has a double period ("enabled.."). This can lead to broken rendering and makes the markup harder to maintain.
<strong>This should launch a new browser window with debugging enabled..</p>
src/mono/wasm/host/DevServer/WebAssemblyNetDebugProxyAppBuilderExtensions.cs:362
- The exception is written into the HTML response without encoding (inside
{ex}). If any part of the exception string contains "<"/"&" (e.g., from a URI or other input), this can become an HTML injection vector. Encode the exception text before writing it into the page.
<h2>Underlying exception:</h2>
<pre>{ex}</pre>
|
Workflow state for the Holistic Review Orchestrator. {
"version": 5,
"last_dispatched_commit": "07b513438a09f104b186a973e0e6d108d482db5b",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "13c03a4a9a854291f77636b1e15038e65919e68b",
"last_reviewed_commit": "07b513438a09f104b186a973e0e6d108d482db5b",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "13c03a4a9a854291f77636b1e15038e65919e68b",
"last_recorded_worker_run_id": "29688143865",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "07b513438a09f104b186a973e0e6d108d482db5b",
"review_id": 4730809954
}
]
} |
There was a problem hiding this comment.
Holistic Review
Motivation: The WASM DevServer debugging help page (Display) writes the url query-string parameter and the computed debugger-tabs list URL directly into an HTML response. Because these values are attacker-influenceable (the url value comes straight from the request query), embedding them unencoded is an HTML/script-injection (reflected XSS) risk on the diagnostic page. The PR closes that gap by HTML-encoding these values before they reach the response.
Approach: The change computes targetApplicationUrlEncoded (via WebUtility.HtmlEncode(targetApplicationUrl.ToString())) and debuggerTabsListUrlEncoded once, then substitutes the encoded variants everywhere the raw values were previously interpolated: the "Unable to find debuggable browser tab" error block (<code> and the Chrome/Edge launch instructions), and the "No inspectable pages found" message. This is consistent with the encoding already applied to tab-derived values elsewhere in the method (tab.Title, tab.Url, devToolsUrlWithProxy). The encoded targetApplicationUrl is correctly used only for display; the raw StringValues is still used for the string.IsNullOrEmpty checks and the ordinal URL match, so behavior is unchanged.
Summary: This is a small, focused, and correct security hardening change with no functional regression. All query-derived values now emitted into the help page are HTML-encoded, and the encoding helper is centralized so the fix is applied consistently. Approving with no actionable findings on the changed lines.
Note: prior Copilot reviews flagged the unencoded <pre>{ex}</pre> exception output. That line is pre-existing code not modified by this PR, so it is out of scope here; if desired it could be addressed in a follow-up, since exception text may transitively contain the same request-derived URL.
Note
This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.
Generated by Holistic Review · 49.7 AIC · ⌖ 10 AIC · ⊞ 10K
Encode query parameters incoming to WebAssembly debugging help page.