Redirect requests with repeated slashes

[dnewbound0]

Let's say you had the following middleware function:

export async function middleware(request: NextRequest) {
  return NextResponse.redirect(new URL("/foo", request.url))
}

A request to https://yourdomain.com//evil.com would result in request.url === "https://evil.com", leading to a malicious redirect. Here we redirect these bad requests containing repeated slashes to a sanitised path.

[changeset-bot]

🦋 Changeset detected

Latest commit: d998bb8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@opennextjs/aws	Patch
app-pages-router	Patch
app-router	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[conico974]

@dnewbound0 I'm sorry but i don't understand what you're trying to solve here.
If you had this in your middleware, it would redirect to https://yourdomain.com/foo, that's the case for me on my test.
And modifying handleRedirects have absolutely no effect since middleware redirect won't go there at all

[dnewbound0]

@dnewbound0 I'm sorry but i don't understand what you're trying to solve here. If you had this in your middleware, it would redirect to https://yourdomain.com/foo, that's the case for me on my test. And modifying handleRedirects have absolutely no effect since middleware redirect won't go there at all

(i've replied to this in the OpenNext discord already but i figured i'd post my response here as well for posterity)

i can recreate this using OpenNext v3.6.1, SST v2.48.5, and Next.js v14.2.28. making a request to http://yourdomain.com//evil.com gives an internal event where the raw path is //evil.com. as far as i understand it that means

opennextjs-aws/packages/open-next/src/core/routing/middleware.ts

Line 70 in daaf38f

const initialUrl = new URL(normalizedPath, internalEvent.url);

evaluates to new URL("//evil.com", "http://yourdomain.com//evil.com") so you're going to a get a middleware invocation where request.url === "http://evil.com".

the PR isn't trying to change any functionality about redirecting within the middleware function, but to avoid calling the middleware entirely with that bad path. so it would go:

1. request to http://yourdomain.com//evil.com
2. before the middleware is invoked, redirect to http://yourdomain.com/evil.com
3. middleware is invoked like normal

as far as i can tell, this is how the default NextJS server handles this case?

apologies if i've missed something obvious here, this is my first PR and i'm just trying to resolve something that i noticed in our deployment 😅

[conico974]

That's basically what Next does
https://github.com/vercel/next.js/blob/3ecf087f10fdfba4426daa02b459387bc9c3c54f/packages/next/src/server/base-server.ts#L1016-L1020

[pkg-pr-new]

Open in StackBlitz

pnpm add https://pkg.pr.new/@opennextjs/aws@863

commit: d998bb8

[conico974]

Just 2 little nit, after that we should be good

[conico974]

Thanks for the PR