🔒️(backend) enhance API input validation to strengthen security
During the bug bounty, attempts were made to pass unexpected hidden fields to manipulate room behavior and join as a ghost. Treat these parameters as suspicious. They are not sent by the frontend, so their presence likely indicates tampering. Explicitly allow the parameters but emit warning logs to help detect and investigate suspicious activity.
This commit is contained in:
committed by
aleb_the_flash
parent
692e0e359e
commit
bfbf253033
@@ -8,6 +8,10 @@ and this project adheres to
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Changed
|
||||
|
||||
- 🔒️(backend) enhance API input validation to strengthen security #1053
|
||||
|
||||
## [1.9.0] - 2026-03-02
|
||||
|
||||
### Added
|
||||
|
||||
@@ -2,9 +2,11 @@
|
||||
|
||||
# pylint: disable=abstract-method,no-name-in-module
|
||||
|
||||
from django.core.exceptions import SuspiciousOperation
|
||||
from django.utils.translation import gettext_lazy as _
|
||||
|
||||
from livekit.api import ParticipantPermission
|
||||
from django_pydantic_field.rest_framework import SchemaField
|
||||
from pydantic import BaseModel, Field
|
||||
from rest_framework import serializers
|
||||
from rest_framework.exceptions import PermissionDenied
|
||||
from timezone_field.rest_framework import TimeZoneSerializerField
|
||||
@@ -261,6 +263,28 @@ class MuteParticipantSerializer(BaseParticipantsManagementSerializer):
|
||||
)
|
||||
|
||||
|
||||
class ParticipantPermission(BaseModel):
|
||||
"""Mirror the LiveKit ParticipantPermission protobuf.
|
||||
|
||||
Control what a participant is allowed to publish, subscribe, and do within a room.
|
||||
Unknown fields are rejected.
|
||||
"""
|
||||
|
||||
can_subscribe: bool | None = None
|
||||
can_publish: bool | None = None
|
||||
can_publish_data: bool | None = None
|
||||
can_publish_sources: list[int] = Field(
|
||||
default_factory=list
|
||||
) # TrackSource enum values
|
||||
hidden: bool | None = None
|
||||
recorder: bool | None = None
|
||||
can_update_metadata: bool | None = None
|
||||
agent: bool | None = None
|
||||
can_subscribe_metrics: bool | None = None
|
||||
|
||||
model_config = {"extra": "forbid"}
|
||||
|
||||
|
||||
class UpdateParticipantSerializer(BaseParticipantsManagementSerializer):
|
||||
"""Validate participant update data."""
|
||||
|
||||
@@ -272,10 +296,11 @@ class UpdateParticipantSerializer(BaseParticipantsManagementSerializer):
|
||||
allow_null=True,
|
||||
help_text="Participant attributes as JSON object",
|
||||
)
|
||||
permission = serializers.DictField(
|
||||
permission = SchemaField(
|
||||
schema=ParticipantPermission | None,
|
||||
required=False,
|
||||
allow_null=True,
|
||||
help_text="Participant permission as JSON object",
|
||||
help_text="Participant permissions",
|
||||
)
|
||||
name = serializers.CharField(
|
||||
max_length=255,
|
||||
@@ -285,6 +310,33 @@ class UpdateParticipantSerializer(BaseParticipantsManagementSerializer):
|
||||
help_text="Display name for the participant",
|
||||
)
|
||||
|
||||
def validate_permission(self, permission):
|
||||
"""Validate that the given permission does not include forbidden or unimplemented fields."""
|
||||
|
||||
if permission is None:
|
||||
return None
|
||||
|
||||
suspicious_fields = [
|
||||
field
|
||||
for field in ("hidden", "recorder", "agent")
|
||||
if getattr(permission, field) is not None
|
||||
]
|
||||
if suspicious_fields:
|
||||
raise SuspiciousOperation(
|
||||
f"Setting the following participant permissions is not allowed: "
|
||||
f"{', '.join(suspicious_fields)}."
|
||||
)
|
||||
if permission.can_subscribe_metrics is not None:
|
||||
raise serializers.ValidationError(
|
||||
{
|
||||
"permission": {
|
||||
"can_subscribe_metrics": "This permission is not implemented."
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
return permission
|
||||
|
||||
def validate(self, attrs):
|
||||
"""Ensure at least one update field is provided."""
|
||||
update_fields = ["metadata", "attributes", "permission", "name"]
|
||||
@@ -300,12 +352,4 @@ class UpdateParticipantSerializer(BaseParticipantsManagementSerializer):
|
||||
f"{', '.join(update_fields)}."
|
||||
)
|
||||
|
||||
if "permission" in attrs:
|
||||
try:
|
||||
ParticipantPermission(**attrs["permission"])
|
||||
except ValueError as e:
|
||||
raise serializers.ValidationError(
|
||||
{"permission": f"Invalid permission: {str(e)}"}
|
||||
) from e
|
||||
|
||||
return attrs
|
||||
|
||||
@@ -607,13 +607,15 @@ class RoomViewSet(
|
||||
serializer = serializers.UpdateParticipantSerializer(data=request.data)
|
||||
serializer.is_valid(raise_exception=True)
|
||||
|
||||
permission = serializer.validated_data.get("permission")
|
||||
|
||||
try:
|
||||
ParticipantsManagement().update(
|
||||
room_name=str(room.pk),
|
||||
identity=str(serializer.validated_data["participant_identity"]),
|
||||
metadata=serializer.validated_data.get("metadata"),
|
||||
attributes=serializer.validated_data.get("attributes"),
|
||||
permission=serializer.validated_data.get("permission"),
|
||||
permission=permission.model_dump() if permission else None,
|
||||
name=serializer.validated_data.get("name"),
|
||||
)
|
||||
except ParticipantsManagementException:
|
||||
|
||||
@@ -8,6 +8,7 @@ import random
|
||||
from unittest import mock
|
||||
from uuid import uuid4
|
||||
|
||||
from django.core.exceptions import SuspiciousOperation
|
||||
from django.urls import reverse
|
||||
|
||||
import pytest
|
||||
@@ -132,11 +133,7 @@ def test_update_participant_success(mock_livekit_client):
|
||||
1,
|
||||
2,
|
||||
], # [TrackSource.CAMERA, TrackSource.MICROPHONE]
|
||||
"hidden": False,
|
||||
"recorder": False,
|
||||
"can_update_metadata": True,
|
||||
"agent": False,
|
||||
"can_subscribe_metrics": False,
|
||||
},
|
||||
"name": "John Doe",
|
||||
}
|
||||
@@ -151,6 +148,151 @@ def test_update_participant_success(mock_livekit_client):
|
||||
mock_livekit_client.aclose.assert_called_once()
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"permission_payload",
|
||||
[
|
||||
{}, # empty dict is valid
|
||||
{"can_subscribe": True},
|
||||
{"can_publish": True},
|
||||
{"can_publish_data": True},
|
||||
{"can_publish_sources": [1, 2]},
|
||||
{"can_update_metadata": True},
|
||||
],
|
||||
)
|
||||
def test_update_participant_permission_fields_are_optional(
|
||||
mock_livekit_client, permission_payload
|
||||
):
|
||||
"""Test that each required permission field can be passed individually."""
|
||||
client = APIClient()
|
||||
room = RoomFactory()
|
||||
user = UserFactory()
|
||||
UserResourceAccessFactory(
|
||||
resource=room, user=user, role=random.choice(["administrator", "owner"])
|
||||
)
|
||||
client.force_authenticate(user=user)
|
||||
|
||||
payload = {
|
||||
"participant_identity": str(uuid4()),
|
||||
"permission": permission_payload,
|
||||
}
|
||||
|
||||
url = reverse("rooms-update-participant", kwargs={"pk": room.id})
|
||||
response = client.post(url, payload, format="json")
|
||||
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
assert response.data == {"status": "success"}
|
||||
|
||||
mock_livekit_client.room.update_participant.assert_called_once()
|
||||
mock_livekit_client.aclose.assert_called_once()
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"value,permission_key",
|
||||
[
|
||||
(False, "hidden"),
|
||||
(True, "hidden"),
|
||||
(False, "recorder"),
|
||||
(True, "recorder"),
|
||||
(False, "agent"),
|
||||
(True, "agent"),
|
||||
],
|
||||
)
|
||||
@mock.patch("core.api.serializers.SuspiciousOperation", side_effect=SuspiciousOperation)
|
||||
def test_update_participant_suspicious_permission(
|
||||
mock_suspicious, value, permission_key
|
||||
):
|
||||
"""Test update participant raises 400 when a restricted permission is set."""
|
||||
client = APIClient()
|
||||
room = RoomFactory()
|
||||
user = UserFactory()
|
||||
UserResourceAccessFactory(
|
||||
resource=room, user=user, role=random.choice(["administrator", "owner"])
|
||||
)
|
||||
client.force_authenticate(user=user)
|
||||
|
||||
payload = {
|
||||
"participant_identity": str(uuid4()),
|
||||
"permission": {
|
||||
"can_subscribe": True,
|
||||
"can_publish": True,
|
||||
"can_publish_data": True,
|
||||
"can_update_metadata": False,
|
||||
permission_key: value,
|
||||
},
|
||||
}
|
||||
|
||||
url = reverse("rooms-update-participant", kwargs={"pk": room.id})
|
||||
response = client.post(url, payload, format="json")
|
||||
|
||||
assert response.status_code == status.HTTP_400_BAD_REQUEST
|
||||
mock_suspicious.assert_called_once_with(
|
||||
f"Setting the following participant permissions is not allowed: {permission_key}."
|
||||
)
|
||||
|
||||
|
||||
@mock.patch("core.api.serializers.SuspiciousOperation", side_effect=SuspiciousOperation)
|
||||
def test_update_participant_suspicious_permission_multiple(mock_suspicious):
|
||||
"""Test update participant raises 400 when multiple suspicious permissions are set."""
|
||||
client = APIClient()
|
||||
room = RoomFactory()
|
||||
user = UserFactory()
|
||||
UserResourceAccessFactory(
|
||||
resource=room, user=user, role=random.choice(["administrator", "owner"])
|
||||
)
|
||||
client.force_authenticate(user=user)
|
||||
|
||||
payload = {
|
||||
"participant_identity": str(uuid4()),
|
||||
"permission": {
|
||||
"can_subscribe": True,
|
||||
"can_publish": True,
|
||||
"can_publish_data": True,
|
||||
"hidden": True,
|
||||
"recorder": False,
|
||||
"can_update_metadata": False,
|
||||
"agent": True,
|
||||
"can_subscribe_metrics": False,
|
||||
},
|
||||
}
|
||||
|
||||
url = reverse("rooms-update-participant", kwargs={"pk": room.id})
|
||||
response = client.post(url, payload, format="json")
|
||||
|
||||
assert response.status_code == status.HTTP_400_BAD_REQUEST
|
||||
mock_suspicious.assert_called_once_with(
|
||||
"Setting the following participant permissions is not allowed: hidden, recorder, agent."
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("value", (False, True))
|
||||
def test_update_participant_unimplemented_can_subscribe_metrics(value):
|
||||
"""Test update participant raises 400 when can_subscribe_metrics is set."""
|
||||
client = APIClient()
|
||||
room = RoomFactory()
|
||||
user = UserFactory()
|
||||
UserResourceAccessFactory(
|
||||
resource=room, user=user, role=random.choice(["administrator", "owner"])
|
||||
)
|
||||
client.force_authenticate(user=user)
|
||||
|
||||
payload = {
|
||||
"participant_identity": str(uuid4()),
|
||||
"permission": {
|
||||
"can_subscribe": True,
|
||||
"can_publish": True,
|
||||
"can_publish_data": True,
|
||||
"can_update_metadata": False,
|
||||
"can_subscribe_metrics": value,
|
||||
},
|
||||
}
|
||||
|
||||
url = reverse("rooms-update-participant", kwargs={"pk": room.id})
|
||||
response = client.post(url, payload, format="json")
|
||||
|
||||
assert response.status_code == status.HTTP_400_BAD_REQUEST
|
||||
assert "can_subscribe_metrics" in str(response.data)
|
||||
|
||||
|
||||
def test_update_participant_forbidden_without_access():
|
||||
"""Test update participant returns 403 when user lacks room privileges."""
|
||||
client = APIClient()
|
||||
@@ -226,7 +368,17 @@ def test_update_participant_invalid_permission():
|
||||
response = client.post(url, payload, format="json")
|
||||
|
||||
assert response.status_code == status.HTTP_400_BAD_REQUEST
|
||||
assert "Invalid permission" in str(response.data)
|
||||
assert response.json() == {
|
||||
"permission": [
|
||||
{
|
||||
"type": "extra_forbidden",
|
||||
"loc": ["invalid-attributes"],
|
||||
"msg": "Extra inputs are not permitted",
|
||||
"input": "True",
|
||||
"url": "https://errors.pydantic.dev/2.12/v/extra_forbidden",
|
||||
},
|
||||
]
|
||||
}
|
||||
|
||||
|
||||
def test_update_participant_wrong_metadata_attributes():
|
||||
|
||||
Reference in New Issue
Block a user