fix(backend): prevent stored XSS via mock server responses and cross-team request moves (#6006)
Co-authored-by: nivedin <nivedinp@gmail.com> Co-authored-by: James George <25279263+jamesgeorge007@users.noreply.github.com>
This commit is contained in:
parent
2fcf5b7a5f
commit
da3b8c5d37
6 changed files with 171 additions and 16 deletions
|
|
@ -103,6 +103,7 @@ export class MockRequestGuard implements CanActivate {
|
|||
// but the mock server ID comes from the subdomain, not the path
|
||||
const subdomainId = this.extractFromSubdomain(host);
|
||||
if (subdomainId) {
|
||||
(request as any).isSubdomainAccess = true;
|
||||
return subdomainId;
|
||||
}
|
||||
|
||||
|
|
@ -111,6 +112,7 @@ export class MockRequestGuard implements CanActivate {
|
|||
// Route pattern: /mock/mock-server-id/...
|
||||
const routeId = this.extractFromRoute(path);
|
||||
if (routeId) {
|
||||
(request as any).isSubdomainAccess = false;
|
||||
return routeId;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -15,6 +15,27 @@ import { MockRequestGuard } from './mock-request.guard';
|
|||
import { MockServer } from 'src/generated/prisma/client';
|
||||
import { ThrottlerBehindProxyGuard } from 'src/guards/throttler-behind-proxy.guard';
|
||||
|
||||
/** Security-sensitive headers that mock responses must not override. */
|
||||
const SECURITY_HEADER_BLOCKLIST = new Set([
|
||||
'content-security-policy',
|
||||
'x-content-type-options',
|
||||
'x-frame-options',
|
||||
'content-disposition',
|
||||
'set-cookie',
|
||||
]);
|
||||
|
||||
/** MIME types that can execute scripts and must be downgraded on same-origin (subpath) responses. */
|
||||
const ACTIVE_CONTENT_TYPES = new Set([
|
||||
'application/javascript',
|
||||
'application/xhtml+xml',
|
||||
'application/xml',
|
||||
'image/svg+xml',
|
||||
'text/html',
|
||||
'text/javascript',
|
||||
'text/xml',
|
||||
'text/xsl',
|
||||
]);
|
||||
|
||||
/**
|
||||
* Mock server controller with dual routing support:
|
||||
* 1. Subdomain pattern: mock-server-id.mock.hopp.io/product
|
||||
|
|
@ -35,6 +56,7 @@ export class MockServerController {
|
|||
// Mock server ID and info are attached by the guard
|
||||
const mockServerId = (req as any).mockServerId as string;
|
||||
const mockServer = (req as any).mockServer as MockServer;
|
||||
const isSubdomainAccess = (req as any).isSubdomainAccess === true;
|
||||
|
||||
if (!mockServerId) {
|
||||
return res.status(HttpStatus.NOT_FOUND).json({
|
||||
|
|
@ -82,14 +104,29 @@ export class MockServerController {
|
|||
|
||||
const mockResponse = result.right;
|
||||
|
||||
// Set response headers if any
|
||||
// Set response headers if any, but exclude security-sensitive headers
|
||||
// that could be abused for XSS
|
||||
if (mockResponse.headers) {
|
||||
try {
|
||||
const headers = JSON.parse(mockResponse.headers);
|
||||
Object.keys(headers).forEach((key) => {
|
||||
console.log('Setting header:', key, headers[key]);
|
||||
res.setHeader(key, headers[key]);
|
||||
});
|
||||
if (
|
||||
headers !== null &&
|
||||
typeof headers === 'object' &&
|
||||
!Array.isArray(headers)
|
||||
) {
|
||||
Object.keys(headers).forEach((key) => {
|
||||
if (!SECURITY_HEADER_BLOCKLIST.has(key.toLowerCase())) {
|
||||
const rawValue = headers[key];
|
||||
// Only allow string and number values to prevent type bypass attacks
|
||||
if (
|
||||
typeof rawValue === 'string' ||
|
||||
typeof rawValue === 'number'
|
||||
) {
|
||||
res.setHeader(key, String(rawValue));
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
} catch (error) {
|
||||
console.error('Error parsing response headers:', error);
|
||||
}
|
||||
|
|
@ -102,6 +139,20 @@ export class MockServerController {
|
|||
);
|
||||
}
|
||||
|
||||
// Prevent XSS on path-based (same-origin) requests by downgrading
|
||||
// script-capable content types to text/plain.
|
||||
// Subdomain-based requests are on a different origin, so HTML is safe.
|
||||
if (!isSubdomainAccess) {
|
||||
const rawContentType = res.getHeader('Content-Type');
|
||||
if (typeof rawContentType === 'string') {
|
||||
// Normalize: trim, strip parameters (e.g. charset), lowercase
|
||||
const mimeType = rawContentType.split(';')[0].trim().toLowerCase();
|
||||
if (ACTIVE_CONTENT_TYPES.has(mimeType) || mimeType.endsWith('+xml')) {
|
||||
res.setHeader('Content-Type', 'text/plain');
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Only set Content-Type if not already set
|
||||
if (!res.getHeader('Content-Type')) {
|
||||
let defaultContentType = 'text/plain';
|
||||
|
|
@ -123,8 +174,15 @@ export class MockServerController {
|
|||
|
||||
res.setHeader('Content-Type', defaultContentType);
|
||||
}
|
||||
// Security headers
|
||||
// Security headers to prevent XSS via mock responses
|
||||
res.setHeader('X-Content-Type-Options', 'nosniff');
|
||||
if (!isSubdomainAccess) {
|
||||
res.setHeader(
|
||||
'Content-Security-Policy',
|
||||
"default-src 'none'; sandbox",
|
||||
);
|
||||
res.setHeader('X-Frame-Options', 'DENY');
|
||||
}
|
||||
|
||||
// Send response
|
||||
return res.status(mockResponse.statusCode).send(mockResponse.body);
|
||||
|
|
|
|||
|
|
@ -481,7 +481,7 @@ describe('findRequestAndNextRequest', () => {
|
|||
nextRequest: dbTeamRequests[4],
|
||||
});
|
||||
});
|
||||
test('Should resolve right if the request and next request null', () => {
|
||||
test('Should resolve right if the request and next request null', async () => {
|
||||
const args: MoveTeamRequestArgs = {
|
||||
srcCollID: teamRequests[0].collectionID,
|
||||
destCollID: teamRequests[4].collectionID,
|
||||
|
|
@ -489,22 +489,65 @@ describe('findRequestAndNextRequest', () => {
|
|||
nextRequestID: null,
|
||||
};
|
||||
|
||||
mockPrisma.teamRequest.findFirst
|
||||
.mockResolvedValueOnce(dbTeamRequests[0])
|
||||
.mockResolvedValueOnce(null);
|
||||
mockPrisma.teamRequest.findFirst.mockResolvedValueOnce(dbTeamRequests[0]);
|
||||
mockPrisma.teamCollection.findUnique.mockResolvedValueOnce(teamCollection);
|
||||
|
||||
const result = (teamRequestService as any).findRequestAndNextRequest(
|
||||
const result = await (teamRequestService as any).findRequestAndNextRequest(
|
||||
args.srcCollID,
|
||||
args.requestID,
|
||||
args.destCollID,
|
||||
args.nextRequestID,
|
||||
);
|
||||
|
||||
expect(result).resolves.toEqualRight({
|
||||
expect(result).toEqualRight({
|
||||
request: dbTeamRequests[0],
|
||||
nextRequest: null,
|
||||
});
|
||||
});
|
||||
test('Should resolve left if the destination collection does not exist when nextRequestID is null', async () => {
|
||||
const args: MoveTeamRequestArgs = {
|
||||
srcCollID: teamRequests[0].collectionID,
|
||||
destCollID: 'non-existent-coll',
|
||||
requestID: teamRequests[0].id,
|
||||
nextRequestID: null,
|
||||
};
|
||||
|
||||
mockPrisma.teamRequest.findFirst.mockResolvedValueOnce(dbTeamRequests[0]);
|
||||
mockPrisma.teamCollection.findUnique.mockResolvedValueOnce(null);
|
||||
|
||||
const result = await (teamRequestService as any).findRequestAndNextRequest(
|
||||
args.srcCollID,
|
||||
args.requestID,
|
||||
args.destCollID,
|
||||
args.nextRequestID,
|
||||
);
|
||||
|
||||
expect(result).toEqualLeft(TEAM_INVALID_COLL_ID);
|
||||
});
|
||||
test('Should resolve left if the destination collection belongs to a different team when nextRequestID is null', async () => {
|
||||
const args: MoveTeamRequestArgs = {
|
||||
srcCollID: teamRequests[0].collectionID,
|
||||
destCollID: 'cross-team-coll',
|
||||
requestID: teamRequests[0].id,
|
||||
nextRequestID: null,
|
||||
};
|
||||
|
||||
mockPrisma.teamRequest.findFirst.mockResolvedValueOnce(dbTeamRequests[0]);
|
||||
mockPrisma.teamCollection.findUnique.mockResolvedValueOnce({
|
||||
...teamCollection,
|
||||
id: 'cross-team-coll',
|
||||
teamID: 'different-team-id',
|
||||
});
|
||||
|
||||
const result = await (teamRequestService as any).findRequestAndNextRequest(
|
||||
args.srcCollID,
|
||||
args.requestID,
|
||||
args.destCollID,
|
||||
args.nextRequestID,
|
||||
);
|
||||
|
||||
expect(result).toEqualLeft(TEAM_REQ_INVALID_TARGET_COLL_ID);
|
||||
});
|
||||
test('Should resolve left if the request is not found', () => {
|
||||
const args: MoveTeamRequestArgs = {
|
||||
srcCollID: teamRequests[0].collectionID,
|
||||
|
|
|
|||
|
|
@ -380,6 +380,18 @@ export class TeamRequestService {
|
|||
) {
|
||||
return E.left(TEAM_REQ_INVALID_TARGET_COLL_ID);
|
||||
}
|
||||
} else {
|
||||
// When nextRequestID is null, validate that the destination collection
|
||||
// belongs to the same team as the request to prevent cross-team moves
|
||||
const destCollection = await this.prisma.teamCollection.findUnique({
|
||||
where: { id: destCollID },
|
||||
select: { teamID: true },
|
||||
});
|
||||
if (!destCollection) return E.left(TEAM_INVALID_COLL_ID);
|
||||
|
||||
if (destCollection.teamID !== request.teamID) {
|
||||
return E.left(TEAM_REQ_INVALID_TARGET_COLL_ID);
|
||||
}
|
||||
}
|
||||
|
||||
return E.right({ request, nextRequest });
|
||||
|
|
|
|||
|
|
@ -137,7 +137,10 @@
|
|||
"description": "Configure mock server settings used to host example responses.",
|
||||
"wildcard_domain": "Wildcard Domain",
|
||||
"wildcard_domain_description": "This field requires a full wildcard domain format. The input must start with an asterisk (*) followed by a dot (.) and then the domain name.",
|
||||
"wildcard_domain_example": "Example: *.mock.domain.com"
|
||||
"wildcard_domain_example": "Example: *.mock.domain.com",
|
||||
"subpath_content_type_notice_prefix": "If you are not using a wildcard domain for the mock server (i.e., using a subpath instead), responses with the following content types are automatically served as",
|
||||
"subpath_content_type_text_plain": "text/plain",
|
||||
"subpath_content_type_notice_suffix": "to help prevent XSS attacks:"
|
||||
},
|
||||
"update_failure": "Failed to update server configurations"
|
||||
},
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@
|
|||
|
||||
<div class="space-y-4 py-4">
|
||||
<div>
|
||||
<label class="block text-sm font-medium text-secondaryDark mb-1">
|
||||
<label class="block text-sm font-medium text-secondaryDark mb-2">
|
||||
{{ t('configs.mock_server.wildcard_domain') }}
|
||||
</label>
|
||||
<HoppSmartInput
|
||||
|
|
@ -24,13 +24,35 @@
|
|||
class="!bg-primaryLight border border-divider rounded"
|
||||
input-styles="!border-0"
|
||||
/>
|
||||
<p class="text-secondaryLight text-sm mt-2">
|
||||
<p class="text-secondaryLight mt-4">
|
||||
{{ t('configs.mock_server.wildcard_domain_description') }}
|
||||
</p>
|
||||
<p class="text-secondaryLight text-sm mt-1 font-mono">
|
||||
<p class="text-secondaryLight mt-1 font-mono font-bold">
|
||||
{{ t('configs.mock_server.wildcard_domain_example') }}
|
||||
</p>
|
||||
</div>
|
||||
|
||||
<div
|
||||
class="flex items-start p-3 bg-primaryLight border border-divider shadow-sm rounded text-sm gap-3"
|
||||
>
|
||||
<icon-lucide-info
|
||||
class="svg-icons text-secondaryLight flex-shrink-0 mt-0.5"
|
||||
/>
|
||||
<div>
|
||||
<p class="text-secondaryDark text-xs">
|
||||
{{ t('configs.mock_server.subpath_content_type_notice_prefix') }}
|
||||
<code class="font-mono font-semibold text-xs text-yellow-500">
|
||||
{{ t('configs.mock_server.subpath_content_type_text_plain') }}
|
||||
</code>
|
||||
{{ t('configs.mock_server.subpath_content_type_notice_suffix') }}
|
||||
</p>
|
||||
<p
|
||||
class="font-mono text-xs text-secondaryLight leading-relaxed mt-1"
|
||||
>
|
||||
{{ sanitizedContentTypes.join(', ') }}
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
|
@ -71,4 +93,19 @@ const mockFields = computed({
|
|||
} else workingConfigs.value.mockServerConfigs.fields = v as any;
|
||||
},
|
||||
});
|
||||
|
||||
// Content types that are served as text/plain when using subpath-based mock URLs (instead of wildcard subdomains) to prevent XSS.
|
||||
// This list should be kept in sync with the backend sanitization logic in
|
||||
// packages/hoppscotch-backend/src/mock-server/mock-server.controller.ts
|
||||
const sanitizedContentTypes = [
|
||||
'application/javascript',
|
||||
'application/xhtml+xml',
|
||||
'application/xml',
|
||||
'*+xml (other +xml subtypes)',
|
||||
'image/svg+xml',
|
||||
'text/html',
|
||||
'text/javascript',
|
||||
'text/xml',
|
||||
'text/xsl',
|
||||
];
|
||||
</script>
|
||||
|
|
|
|||
Loading…
Reference in a new issue