diff --git a/src/backend/InvenTree/InvenTree/api.py b/src/backend/InvenTree/InvenTree/api.py index c08f24289c..cd18e2a7b3 100644 --- a/src/backend/InvenTree/InvenTree/api.py +++ b/src/backend/InvenTree/InvenTree/api.py @@ -877,6 +877,9 @@ class GenericMetadataView(RetrieveUpdateAPI): serializer_class = MetadataSerializer permission_classes = [InvenTree.permissions.ContentTypePermission] + # Enforce limited range of lookup fields to prevent arbitrary queryset filtering + ALLOWED_LOOKUP_FIELDS = {'pk', 'key'} + def get_permission_model(self): """Return the 'permission' model associated with this view.""" model_name = self.kwargs.get('model', None) @@ -921,6 +924,17 @@ class GenericMetadataView(RetrieveUpdateAPI): ) return super().dispatch(request, *args, **kwargs) + def initial(self, request, *args, **kwargs): + """Validate the lookup field before any queryset is touched. + + This runs inside APIView's own exception handling (unlike dispatch(), + which runs before it), and *before* permission checks - so an invalid + lookup field is rejected without ever executing a query. + """ + if self.lookup_field not in self.ALLOWED_LOOKUP_FIELDS: + raise ValidationError(f"Invalid lookup field '{self.lookup_field}'") + return super().initial(request, *args, **kwargs) + class SimpleGenericMetadataView(GenericMetadataView): """Simplified version of GenericMetadataView which always uses 'pk' as the lookup field.""" diff --git a/src/backend/InvenTree/InvenTree/test_api.py b/src/backend/InvenTree/InvenTree/test_api.py index c74632d4bc..1f67579881 100644 --- a/src/backend/InvenTree/InvenTree/test_api.py +++ b/src/backend/InvenTree/InvenTree/test_api.py @@ -4,6 +4,7 @@ from base64 import b64encode from pathlib import Path from tempfile import TemporaryDirectory +from django.contrib.auth import get_user_model from django.core.exceptions import AppRegistryNotReady from django.test import TestCase from django.urls import reverse @@ -15,6 +16,7 @@ from InvenTree.api_version import INVENTREE_API_VERSION from InvenTree.exceptions import exception_handler from InvenTree.unit_test import InvenTreeAPITestCase, InvenTreeTestCase from InvenTree.version import inventreeApiText, parse_version_text +from users.models import ApiToken from users.ruleset import RULESET_NAMES from users.tasks import update_group_roles @@ -658,3 +660,56 @@ class GeneralApiTests(InvenTreeAPITestCase): self.assertIn('bom-exporter', keys) self.assertIn('inventree-ui-notification', keys) self.assertIn('inventreelabel', keys) + + def test_generic_metadata_lookup_field_injection(self): + """The generic metadata endpoint must not accept arbitrary lookup expressions. + + Regression test for a vulnerability where 'lookup_field' was passed + straight from the URL into the ORM (e.g. 'key__regex'), letting an + unprivileged user turn distinguishable 403 / 404 / 500 responses into + an oracle to recover another user's raw API token, byte by byte. + """ + victim = get_user_model().objects.create_user( + username='metadata_victim', password='hunter2', email='victim@example.org' + ) + token = ApiToken.objects.create(user=victim) + + def lookup_url(model, lookup_field, lookup_value): + return reverse( + 'api-generic-metadata', + kwargs={ + 'model': model, + 'lookup_field': lookup_field, + 'lookup_value': lookup_value, + }, + ) + + # A pattern which *would* match the victim's token if 'key__regex' + # were actually applied as a regex filter against ApiToken.key + matching_pattern = f'^{token.key}$' + non_matching_pattern = '^this-will-never-match-anything$' + + responses = set() + + for pattern in (matching_pattern, non_matching_pattern): + for lookup_field in ('key__regex', 'key__contains', 'key__startswith'): + response = self.get( + lookup_url('apitoken', lookup_field, pattern), expected_code=400 + ) + self.assertIn('Invalid lookup field', str(response.data)) + responses.add(response.status_code) + + # Matching and non-matching patterns must be indistinguishable + self.assertEqual(len(responses), 1) + + # The exact-match 'key' lookup stays permitted (used elsewhere, e.g. + # by plugin config lookups) - a non-admin still can't read another + # user's token metadata through it, since object permissions apply. + self.get(lookup_url('apitoken', 'key', token.key), expected_code=403) + + # Not apitoken-specific: any model/lookup-type combination outside + # the allow-list is rejected the same way. + response = self.get( + lookup_url('user', 'password__startswith', 'x'), expected_code=400 + ) + self.assertIn('Invalid lookup field', str(response.data))