Skip to content

fix(@angular/build): escape hostname in host-validation 403 error page - #33673

Closed
deprrous wants to merge 1 commit into
angular:mainfrom
deprrous:fix/host-check-xss-escape
Closed

fix(@angular/build): escape hostname in host-validation 403 error page#33673
deprrous wants to merge 1 commit into
angular:mainfrom
deprrous:fix/host-check-xss-escape

Conversation

@deprrous

Copy link
Copy Markdown

Problem

The html403() function in host-check-middleware.ts interpolates req.headers.host directly into an HTML response without entity encoding:

const hostname = req.headers.host?.toLowerCase().split(':')[0] ?? '';
res.end(html403(hostname));
// ...
`<h1>Blocked request. This host ("${hostname}") is not allowed.</h1>`

The response is served with Content-Type: text/html. A raw HTTP request with HTML metacharacters in the Host header reflects unescaped content.

Reproduction

ng serve --host 0.0.0.0 --port 4200
curl -s -H 'Host: <img src=x onerror=alert(document.domain)>' http://127.0.0.1:4200/

Response body contains the unescaped payload in a text/html response.

Fix

Add escapeHtml() to encode &, <, >, and " before interpolation into the HTML template.

The Host header value was interpolated directly into the HTML response
without entity encoding. While browsers set Host automatically (preventing
direct exploitation), a raw HTTP request with HTML metacharacters in the
Host header would reflect unescaped content in a text/html response.

Add escapeHtml() to encode &, <, >, and " before interpolation.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces an escapeHtml utility function to sanitize the hostname displayed in the 403 HTML error page of the host check middleware, mitigating potential HTML injection. The reviewer suggested optimizing this function by using a single regular expression with a character map lookup instead of chained .replace calls, and extending it to escape single quotes (') to ensure robust XSS prevention.

Comment on lines +45 to +51
function escapeHtml(value: string): string {
return value
.replace(/&/g, '&amp;')
.replace(/</g, '&lt;')
.replace(/>/g, '&gt;')
.replace(/"/g, '&quot;');
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation of escapeHtml uses multiple chained .replace calls with regular expressions, which results in multiple string allocations and full-string scans. Additionally, it does not escape the single quote ('), which is a standard HTML entity that should be escaped to prevent potential XSS vulnerabilities in attribute contexts.

Using a single regular expression with a character class and a map lookup is more efficient and allows us to easily include the single quote (') in the escaped characters.

function escapeHtml(value: string): string {
  const entityMap: Record<string, string> = {
    '&': '&amp;',
    '<': '&lt;',
    '>': '&gt;',
    '"': '&quot;',
    "'": '&#39;',
  };
  return value.replace(/[&<>"']/g, (s) => entityMap[s]);
}

@alan-agius4

Copy link
Copy Markdown
Collaborator

Thank you for reporting this and submitting a PR.

We are closing this as it is not an exploitable security vulnerability. For a reflected XSS attack to be exploitable against a victim, an attacker must be able to trick a victim's browser into sending the malicious payload to the server.

Since ng serve is a local development server and an attacker cannot induce a victim's browser to send a crafted Host header containing HTML metacharacters, this scenario does not present an exploitable security risk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants