Skip to content

Open redirect regression in res.location() — reintroduces CVE-2024-29041 [Severity: Medium] #7480

Description

@outhackernuls090-hash

Hi,
I'm reporting what looks like a regression of a vulnerability you already fixed once. While auditing a copy of express@5.2.1, I found that lib/response.js's res.location() is back to the pre-patch implementation from CVE-2024-29041 / GHSA-rv95-896h-c2vc, the open redirect issue you fixed in 4.19.2 and 5.0.0-beta.3 back in March 2024.

Here's what's shipping:

res.location = function location(url) {
  return this.set('Location', encodeUrl(url));
};

That's exactly the version the original advisory describes as vulnerable — a single, unguarded encodeUrl(url) call with none of the scheme/host-splitting logic your fix added. For comparison, here's what the patched version looks like (from your own commits 0867302 and 0b74695):

res.location = function location(url) {
  var loc;
  if (url === 'back') {
    loc = this.req.get('Referrer') || '/';
  } else {
    loc = String(url);
  }
  var m = schemaAndHostRegExp.exec(loc);
  var pos = m ? m[0].length + 1 : 0;
  loc = loc.slice(0, pos) + encodeUrl(loc.slice(pos));
  return this.set('Location', loc);
};

I don't know how this happened on your end — maybe a bad revert, a packaging mistake, a stray build script pulling from an old branch — but as far as I can tell every genuinely published 5.2.1 on npm has the fix, and this copy express-5.2.1.zip doesn't. Flagging it in case it points to something in the release pipeline worth double-checking, and because if a build like this ever does go out, it's a real, exploitable bug.

Why it's still exploitable today

encodeUrl (your dependency on the encodeurl package) is intentionally conservative about what it touches. Its allow-list regex treats the byte range 0x3F–0x5F as already safe, and a raw backslash (0x5C) sits right in that range. So encodeUrl('/\\evil.com') comes back as /\evil.com, untouched.

That matters because browsers don't treat that as harmless. Per the WHATWG URL Standard, a backslash is equivalent to a forward slash for special schemes like http and https. So a Location header of /\evil.com/phish — which looks like a safe, same-site relative path — gets resolved by the browser to //evil.com/phish, a full protocol-relative redirect to a different host. I checked this against Node's own URL parser, which implements the same algorithm:

new URL('/\\evil.com/phish', 'https://victim.example.com/app/redirect').href
// "https://evil.com/phish"

A lot of apps guard redirect targets with something like:

if (target.startsWith('/') && !target.startsWith('//')) {
  res.redirect(target);
}

figuring that a single leading slash means "same-origin, nothing to worry about." That check passes for /\evil.com/phish, and since res.location() doesn't re-parse or validate anything, the attacker's bytes go straight into the header. That's a working bypass of a very common, otherwise-reasonable defense.

Proof of concept

I've attached a small, dependency-free Node script (poc_open_redirect.js) that pulls res.location() verbatim out of the affected response.js, wires it up behind exactly the allow-list pattern above, and fires a real HTTP request at it. Run it with:

node poc_open_redirect.js path/to/express/lib/response.js

Output from an actual run:

Attacker-supplied "next" parameter : "/\\evil.com/phish"
App allow-list check               : PASSED (starts with "/", not "//")
HTTP status from server             : 302
Location header actually sent       : "/\\evil.com/phish"

Browser-resolved destination (WHATWG URL -- same algorithm Chrome/Firefox use):
  -> http://evil.com/phish
  -> resolved host: evil.com

[VULNERABLE] The "/"-only allow-list was bypassed: the victim's browser is
sent to evil.com even though the server-side check only permits same-site paths.

I also included encodeurl_impl.js, a small standalone reimplementation of the encodeurl package's encoding rules, which I used to independently confirm the backslash behavior without needing the real dependency installed.

Suggested fix

Just restore the patched implementation shown above (the scheme/host split before encoding). If it's easier, I'd also lean toward pointing people at the later refinement you shipped after the initial fix had its own bypass — the version that double-checks the parsed host hasn't changed after encoding — since that's a stronger guarantee than the regex split alone.

Everything else I checked was fine

While I was in there I looked over the rest of lib/ for anything similar. Nothing else stood out:

  • The JSONP callback sanitization and the "/**/ typeof ... === 'function'" Rosetta Flash mitigation in res.jsonp() match current behavior.
  • res.redirect()'s default HTML body still escapes the address with escapeHtml(), so the September 2024 XSS fix (GHSA-qw6h-vgh9-j6wx) is intact.
  • res.status() has the stricter integer/range validation you added more recently.
  • The "extended" query parser hardcodes qs.parse(str, { allowPrototypes: true }), which looked worrying at first glance, but I tested it directly against a real qs build and couldn't get prototype pollution out of it — qs strips literal __proto__ keys regardless of that flag, and the parsed result only ever gets object-spread into app.render()'s options, never merged in a way that reaches the real prototype. Also worth noting this parser isn't even the default in v5 anymore.
  • req.protocol, req.ip/req.ips, and req.host/req.hostname all match the current trust-proxy handling.
  • view.js's path resolution is the same as upstream; passing untrusted input as a view name is a known, documented risk that lives at the application layer, not something specific to this build.

Also I want to mention that I used AI for this report aswell and its not fully human made.

encodeurl_impl.js
poc_open_redirect.js

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions