Skip to content

fix(security): constrain WHIP/WHEP path params to prevent log injection #306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
birme wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(security): constrain WHIP/WHEP path params to prevent log injection #306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/api_whep.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,12 @@ export const apiWhep: FastifyPluginCallback<ApiWhepOptions> = (
schema: {
description: 'WHEP endpoint for Egress WebRTC streams',
params: Type.Object({
productionId: Type.String({ maxLength: 200 }),
lineId: Type.String({ maxLength: 200 }),
username: Type.String({ maxLength: 200 })
productionId: Type.String({ minLength: 1, pattern: '^[0-9]+$' }),
lineId: Type.String({ minLength: 1, pattern: '^[0-9]+$' }),
username: Type.String({
maxLength: 200,
pattern: '^[\\w .-]{1,200}$'
})
}),
body: WhipWhepRequest,
response: {
Expand Down
58 changes: 44 additions & 14 deletions src/api_whip.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ describe('apiWhip', () => {

const response = await fastify.inject({
method: 'POST',
url: '/whip/prod1/line1/testuser',
url: '/whip/123/456/testuser',
headers: {
'content-type': 'application/sdp'
},
Expand All @@ -156,7 +156,7 @@ describe('apiWhip', () => {
expect(response.statusCode).toBe(201);
expect(response.headers['content-type']).toBe('application/sdp');
expect(response.headers['location']).toContain(
'/whip/prod1/line1/mock-session-id'
'/whip/123/456/mock-session-id'
);
expect(response.payload).toContain('v=0');
});
Expand All @@ -170,7 +170,7 @@ describe('apiWhip', () => {

const response = await fastify.inject({
method: 'POST',
url: '/whip/prod1/line1/testuser',
url: '/whip/123/456/testuser',
headers: {
'content-type': 'application/sdp'
},
Expand All @@ -192,7 +192,7 @@ describe('apiWhip', () => {

const response = await fastify.inject({
method: 'POST',
url: '/whip/prod1/line1/testuser',
url: '/whip/123/456/testuser',
headers: {
'content-type': 'application/json'
},
Expand All @@ -202,14 +202,44 @@ describe('apiWhip', () => {
expect(response.statusCode).toBe(415);
});

it('should return 400 when username contains control chars (log injection)', async () => {
const fastify = await createTestServer();

const response = await fastify.inject({
method: 'POST',
url: '/whip/123/456/' + encodeURIComponent('evil\ninjected'),
headers: {
'content-type': 'application/sdp'
},
payload: 'v=0\r\n'
});

expect(response.statusCode).toBe(400);
});

it('should return 400 when productionId is not numeric', async () => {
const fastify = await createTestServer();

const response = await fastify.inject({
method: 'POST',
url: '/whip/abc/456/testuser',
headers: {
'content-type': 'application/sdp'
},
payload: 'v=0\r\n'
});

expect(response.statusCode).toBe(400);
});

it('should return 429 when rate limit is exceeded', async () => {
const fastify = await createTestServer();

// Send 10 valid requests (these should succeed or at least not trigger 429)
for (let i = 0; i < 10; i++) {
await fastify.inject({
method: 'POST',
url: '/whip/prod1/line1/testuser',
url: '/whip/123/456/testuser',
headers: {
'content-type': 'application/sdp'
},
Expand All @@ -220,7 +250,7 @@ describe('apiWhip', () => {
// The 11th request should exceed the rate limit
const response = await fastify.inject({
method: 'POST',
url: '/whip/prod1/line1/testuser',
url: '/whip/123/456/testuser',
headers: {
'content-type': 'application/sdp'
},
Expand All @@ -241,7 +271,7 @@ describe('apiWhip', () => {
const fastify = await createAuthServer();
const res = await fastify.inject({
method: 'POST',
url: '/whip/prod1/line1/testuser',
url: '/whip/123/456/testuser',
headers: { 'content-type': 'application/sdp' },
payload: 'v=0\r\n'
});
Expand All @@ -253,7 +283,7 @@ describe('apiWhip', () => {
const fastify = await createAuthServer();
const res = await fastify.inject({
method: 'POST',
url: '/whip/prod1/line1/testuser',
url: '/whip/123/456/testuser',
headers: {
'content-type': 'application/sdp',
authorization: 'Bearer wrong'
Expand All @@ -268,7 +298,7 @@ describe('apiWhip', () => {
const fastify = await createAuthServer();
const res = await fastify.inject({
method: 'POST',
url: '/whip/prod1/line1/testuser',
url: '/whip/123/456/testuser',
headers: {
'content-type': 'application/sdp',
authorization: 'Bearer secret-123'
Expand All @@ -285,7 +315,7 @@ describe('apiWhip', () => {
const fastify = await createAuthServer();
const res = await fastify.inject({
method: 'DELETE',
url: '/whip/prod1/line1/mock-session-id'
url: '/whip/123/456/mock-session-id'
});
expect(res.statusCode).toBe(401);
});
Expand All @@ -294,7 +324,7 @@ describe('apiWhip', () => {
const fastify = await createAuthServer();
const res = await fastify.inject({
method: 'DELETE',
url: '/whip/prod1/line1/mock-session-id',
url: '/whip/123/456/mock-session-id',
headers: { authorization: 'Bearer secret-123' }
});
expect(res.statusCode).toBe(200);
Expand All @@ -309,7 +339,7 @@ describe('apiWhip', () => {

const response = await fastify.inject({
method: 'DELETE',
url: '/whip/prod1/line1/mock-session-id'
url: '/whip/123/456/mock-session-id'
});

expect(response.statusCode).toBe(200);
Expand All @@ -322,7 +352,7 @@ describe('apiWhip', () => {

const response = await fastify.inject({
method: 'DELETE',
url: '/whip/prod1/line1/nonexistent-session'
url: '/whip/123/456/nonexistent-session'
});

expect(response.statusCode).toBe(404);
Expand Down Expand Up @@ -378,7 +408,7 @@ describe('apiWhip', () => {

const response = await fastify.inject({
method: 'PATCH',
url: '/whip/prod1/line1/mock-session-id',
url: '/whip/123/456/mock-session-id',
headers: { 'content-type': 'application/trickle-ice-sdpfrag' },
payload: 'a=candidate:1 1 UDP 12345 192.168.1.2 54321 typ host'
});
Expand Down
9 changes: 6 additions & 3 deletions src/api_whip.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,12 @@ export const apiWhip: FastifyPluginCallback<ApiWhipOptions> = (
schema: {
description: 'WHIP endpoint for ingesting WebRTC streams',
params: Type.Object({
productionId: Type.String({ maxLength: 200 }),
lineId: Type.String({ maxLength: 200 }),
username: Type.String({ maxLength: 200 })
productionId: Type.String({ minLength: 1, pattern: '^[0-9]+$' }),
lineId: Type.String({ minLength: 1, pattern: '^[0-9]+$' }),
username: Type.String({
maxLength: 200,
pattern: '^[\\w .-]{1,200}$'
})
}),
body: WhipWhepRequest,
response: {
Expand Down
Loading