isom-2008 feat(redirect): enhance checks for bypassing transition page

[adriangohjw]

Problem

What problem are you trying to solve? What issue does this close?

Closes https://linear.app/ogp/issue/ISOM-2008/vapt-insufficient-referrer-validation-bypasses-transition-page

Solution

How did you solve the problem?

Improvements:

• use built-in URL function to check instead of startsWith
• extract into private method for readability
• add tests

Tests

What tests should be run to confirm functionality?

• refer to Linear ticket for steps to reproduce

[linear]

ISOM-2008 [VAPT] Insufficient referrer validation bypasses transition page

[adriangohjw]

bugbot run

[adriangohjw]

Bug: Trusted Referrer Tests Expect Incorrect Redirect

The RedirectService.test.ts file contains incorrect expectations for trusted referrer scenarios. Tests for referrers from the same origin as ogUrl (e.g., exact match, with path, or with query parameters) currently expect RedirectType.TransitionPage. However, the RedirectService's logic dictates RedirectType.Direct for these cases, as a trusted referrer should bypass the transition page.

fixed tests in 29b40c8

[adriangohjw]

Bug: Malformed URL in Security Test

The test "should NOT allow transition page bypass for domain containing ogUrl" constructs a malformed referrer URL. The template literal https://staging.${ogUrl}.malicious.com results in https://staging.https://go.gov.sg.malicious.com because ogUrl already includes https://, creating a double protocol. While the test passes (as invalid URLs are correctly treated as untrusted), it does not test the intended scenario of a malicious domain containing the trusted domain name. The URL should be https://staging.go.gov.sg.malicious.com.

fixed in 9206094

[dcshzj]

issue: should use this instead of RedirectService

[adriangohjw]

actually im not super familiar with the framework, but i referenced this from an existing code

GoGovSG/src/server/modules/redirect/services/RedirectService.ts

Lines 47 to 49 in 2d94333

	if (RedirectService.isValidShortUrl(rawShortUrl)) {
	throw new NotFoundError('Invalid Url')
	}

GoGovSG/src/server/modules/redirect/services/RedirectService.ts

Lines 98 to 100 in 2d94333

	private static isValidShortUrl(shortUrl: string): boolean {
	return !shortUrl || !/^[a-zA-Z0-9-]+$/.test(shortUrl)
	}

curious - what's the issue for this?

[dcshzj]

Ah oops sorry was reviewing on mobile, so didn't see the static part. Doing RedirectService.isFromTrustedPage is ok since isFromTrustedPage is a static function. If otherwise then should use this.

[dcshzj]

nit: can name it after the function instead? Easier to identify and debug

[adriangohjw]

updated in 36b91b0 👍

[dcshzj]

suggestion: I think this init can be outside of the beforeEach and describe, might help in making tests run a bit faster

[adriangohjw]

updated in 1684f26 👍

[dcshzj]

Ah oops sorry was reviewing on mobile, so didn't see the static part. Doing RedirectService.isFromTrustedPage is ok since isFromTrustedPage is a static function. If otherwise then should use this.