Skip to content

fix(platform-server): prevent SSRF bypasses via backslash URLs in HttpClient#68845

Open
JeanMeche wants to merge 2 commits into
angular:mainfrom
JeanMeche:server-backslash
Open

fix(platform-server): prevent SSRF bypasses via backslash URLs in HttpClient#68845
JeanMeche wants to merge 2 commits into
angular:mainfrom
JeanMeche:server-backslash

Conversation

@JeanMeche
Copy link
Copy Markdown
Member

Encoding backslashes ensures that they are not normalized to slashes and where they could generate a protocol relative URL.

…pClient

Encoding backslashes ensures that they are not normalized to slashes and where they could generate a protocol relative URL.
@JeanMeche JeanMeche requested a review from alan-agius4 May 20, 2026 23:05
@angular-robot angular-robot Bot added the area: server Issues related to server-side rendering label May 20, 2026
@ngbot ngbot Bot added this to the Backlog milestone May 20, 2026
});
});

it('prevents SSRF bypasses via backslash URLs in HttpClient', async () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This test only covers a small part. Can you add some more included cases that should pass.

        it('should allow legitimate protocol-relative URLs', async () => {
          ref.injector.get(NgZone).run(() => {
            http.get('//example.com/testing').subscribe((body) => {
              expect(body).toEqual('success!');
            });
            mock.expectOne('http://example.com/testing').flush('success!');
          });
        });

        it('should treat backslash bypass SSRF attempts in relative requests strictly as pathnames', async () => {
          const badUrls = [
            '/\\attacker.com',
            '\\\\attacker.com',
            '///attacker.com',
            '//\\attacker.com',
            '  /\\attacker.com',
            '\r\n/\\attacker.com',
          ];

          ref.injector.get(NgZone).run(() => {
            for (const badUrl of badUrls) {
              http.get(badUrl).subscribe((body) => {
                expect(body).toEqual('success!');
              });
              mock.expectOne('http://localhost:4000/attacker.com').flush('success!');
            }
          });
        });

        it('should resolve safe path-relative URLs containing backslashes without origin change', async () => {
          ref.injector.get(NgZone).run(() => {
            http.get('\\testing').subscribe((body) => {
              expect(body).toEqual('success!');
            });
            mock.expectOne('http://localhost:4000/testing').flush('success!');
          });
        });

        it('should resolve backslashes inside path-relative segments without origin change', async () => {
          ref.injector.get(NgZone).run(() => {
            http.get('/foo\\bar').subscribe((body) => {
              expect(body).toEqual('success!');
            });
            mock.expectOne('http://localhost:4000/foo/bar').flush('success!');
          });
        });

        it('should resolve relative request URLs without leading slash relative to parent path', async () => {
          ref.injector.get(NgZone).run(() => {
            http.get('testing').subscribe((body) => {
              expect(body).toEqual('success!');
            });
            mock.expectOne('http://localhost:4000/testing').flush('success!');
          });
        });
      });

      describe(`given 'url' is provided in 'INITIAL_CONFIG' with a trailing slash`, () => {
        let mock: HttpTestingController;
        let ref: NgModuleRef<HttpInterceptorExampleModule>;
        let http: HttpClient;

        beforeEach(async () => {
          const platform = platformServer([
            {
              provide: INITIAL_CONFIG,
              useValue: {
                document: '<app></app>',
                url: 'http://localhost:4000/foo/',
              },
            },
          ]);

          ref = await platform.bootstrapModule(HttpInterceptorExampleModule);
          mock = ref.injector.get(HttpTestingController);
          http = ref.injector.get(HttpClient);
        });

        it('should resolve sub-path relative request URLs relative to trailing-slash base URL', async () => {
          ref.injector.get(NgZone).run(() => {
            http.get('testing').subscribe((body) => {
              expect(body).toEqual('success!');
            });
            mock.expectOne('http://localhost:4000/foo/testing').flush('success!');
          });
        });

Comment thread packages/platform-server/src/http.ts Outdated
// The `URL` constructor normalizes backslashes to forward slashes, which can
// cause `/\` to be evaluated as a protocol-relative URL (`//`).
// Replacing backslashes with their URL-encoded equivalent prevents this.
const safeUrl = request.url.replace(/\\/g, '%5C');
Copy link
Copy Markdown
Contributor

@alan-agius4 alan-agius4 May 21, 2026

Choose a reason for hiding this comment

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

While this isn't a massive issue, it forces all backslashes to be encode, even those that aren't meant to be protocol-relative. For instance, an input like foo\\bar gets parsed into a pathname of /foo%5Cbar, which alters expected URL parsing behavior and could break legitimate uses.

Additionally, this doesn't catch forward slashes //, which are also treated as protocol-relative by new URL(). An input like //foo would still bypass this check and leave us vulnerable.

I think a better approach would look something like this:

// Standardizing backslashes to forward slashes ensures the URL constructor resolves it properly.
// This is the same behaviour of new URL
const originalUrl = request.url.trim();
let url = originalUrl.replace(/\\/g, '/');

// If it looks like a relative path, ensure it starts with a single forward slash.
// We check legitimacy on the originalUrl before backslash substitution, ensuring that relative bypass
// attempts (like '/\\' or '\\\\') are treated strictly as paths rather than being converted to protocol-relative targets.
const isAbsolute = URL.canParse(originalUrl);
const isLegitProtocolRelative = originalUrl.startsWith('//') && originalUrl[2] !== '/' && originalUrl[2] !== '\\';

if (!isAbsolute && !isLegitProtocolRelative) {
  if (originalUrl[0] === '/' || originalUrl[0] === '\\') {
    url = '/' + url.replace(/^\/+/, '');
  }
}

const newUrl = new URL(url, baseUrl).toString();

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

Labels

area: server Issues related to server-side rendering

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants