location function does not validate potentially malicious URLs

[xzyingxiashubro]

Bug: location function does not validate potentially malicious URLs

Environment information

Version: express 5.0.0-beta.3

Platform: Linux

Node.js version: 18.x

Any other relevant information: N/A

What steps will reproduce the bug?

1. Create an Express application
2. Set up a route that uses res.location() with a malicious URL
3. Send a request to this route and observe the response headers

const express = require('express');
const app = express();

app.get('/redirect', (req, res) => {
  // Set a malicious URL in the Location header
  res.location('javascript:alert("XSS attack")');
  res.status(302).send('Redirecting...');
});

app.listen(3000, () => {
  console.log('Server running on port 3000');
});

// Using curl to test:
// $ curl -i http://localhost:3000/redirect
//
// The response headers will contain:
// Location: javascript:alert("XSS attack")

What is the expected behavior?

The location function should validate the URL scheme and reject or sanitize non-HTTP(S) URLs, especially those with potentially dangerous schemes like javascript:, data.

What happens instead?

The function sets the malicious URL directly in the Location header without any validation:

HTTP/1.1 302 Found
Location: javascript:alert("XSS attack")
...

This creates a security vulnerability as browsers following this redirect might execute the JavaScript code, leading to Cross-Site Scripting (XSS) attacks.

Source of the issue

The issue is in the location function in lib/response.js.

[SantiagoCecena]

Hi,

I'm interested in working on this issue.

The current behavior of res.location() allowing javascript: or data: schemes in the Location header poses a significant XSS vulnerability, as browsers might execute such malicious URLs. While encodeUrl handles URL encoding, it doesn't validate the safety of the URL scheme itself.

I'd like to propose adding a scheme validation step within the res.location function. This would involve checking if the provided URL starts with safe schemes like http://, https://, or is a relative path (/). Any other schemes (e.g., javascript:, data:) would be rejected or handled safely to prevent this type of attack.

I believe this enhancement would significantly improve the default security of Express for all users.

Would it be possible for me to pick this up and submit a pull request? I'm happy to discuss the implementation details further.

Thanks,
Santiago

[ctcpip]

Hi, please note our threat model is clear that it is the application's responsibility to control what is passed to methods, including res.location(). Furthermore, there are perfectly valid use cases for non http schemes in location headers.