CVE-2023-46128: Cleartext Storage of Sensitive Information (CWE-312)
value: Nautobot is a Network Automation Platform built as a web application atop the Django Python framework with a PostgreSQL or MySQL database. In Nautobot 2.0.x, certain REST API endpoints, in combination with the `?depth=<N>` query parameter, can expose hashed user passwords as stored in the database to any authenticated user with access to these endpoints. The passwords are not exposed in plaintext. This vulnerability has been patched in version 2.0.3.


CWE description: The product stores sensitive information in cleartext within a resource that might be accessible to another control sphere.
CVSS v3 base score: 6.5
Published: 2023-10-25T18:17Z
Repository: https://github.com/nautobot/nautobot
Language: Python
Files changed: changes/4692.fixed, changes/4692.housekeeping, changes/4692.security, nautobot/core/api/utils.py, nautobot/core/testing/api.py, nautobot/docs/development/apps/migration/code-updates.md
Commit date: 2023-10-23 16:56:09 -0400
Commit message: Fix `Meta` inheritance in nested serializer classes (#4692)

* Add test coverage

* Fix issue

* Change fragment

* Add more change fragments

* Refine test implementation

* Add reference to GHSA

* Add admonition about use of fields = __all__ on serializers

* Add PR number
Diff: diff --git a/changes/4692.fixed b/changes/4692.fixed
new file mode 100644
index 000000000..29df631ac
--- /dev/null
+++ b/changes/4692.fixed
@@ -0,0 +1 @@
+Fixed incorrect inheritance of `Meta` attributes into nested serializers (`depth >= 1`).
diff --git a/changes/4692.housekeeping b/changes/4692.housekeeping
new file mode 100644
index 000000000..dd263638a
--- /dev/null
+++ b/changes/4692.housekeeping
@@ -0,0 +1 @@
+Added check in REST API generic test cases to detect strings like `password` and `sha256` that shouldn't generally appear in REST API responses.
diff --git a/changes/4692.security b/changes/4692.security
new file mode 100644
index 000000000..699d064e9
--- /dev/null
+++ b/changes/4692.security
@@ -0,0 +1 @@
+Fixed potential exposure of hashed user password data on certain REST API endpoints when using the `?depth=1` query parameter. For more details, please refer to [GHSA-r2hw-74xv-4gqp](https://github.com/nautobot/nautobot/security/advisories/GHSA-r2hw-74xv-4gqp).
diff --git a/nautobot/core/api/utils.py b/nautobot/core/api/utils.py
index ec2b83081..c38238850 100644
--- a/nautobot/core/api/utils.py
+++ b/nautobot/core/api/utils.py
@@ -312,18 +312,13 @@ def nested_serializer_factory(relation_info, nested_depth):
         field_kwargs = get_nested_relation_kwargs(relation_info)
     else:
         base_serializer_class = get_serializer_for_model(relation_info.related_model)
 
         class NautobotNestedSerializer(base_serializer_class):
-            class Meta:
-                model = relation_info.related_model
+            class Meta(base_serializer_class.Meta):
                 is_nested = True
                 depth = nested_depth - 1
-                if hasattr(base_serializer_class.Meta, "fields"):
-                    fields = base_serializer_class.Meta.fields
-                if hasattr(base_serializer_class.Meta, "exclude"):
-                    exclude = base_serializer_class.Meta.exclude
 
         NautobotNestedSerializer.__name__ = nested_serializer_name
         NESTED_SERIALIZER_CACHE[nested_serializer_name] = NautobotNestedSerializer
         field_class = NautobotNestedSerializer
         field_kwargs = get_nested_relation_kwargs(relation_info)
diff --git a/nautobot/core/testing/api.py b/nautobot/core/testing/api.py
index 4db221ede..3830bd245 100644
--- a/nautobot/core/testing/api.py
+++ b/nautobot/core/testing/api.py
@@ -71,10 +71,35 @@ class APITestCase(views.ModelTestCase):
 
     def _get_list_url(self):
         viewname = lookup.get_route_for_model(self.model, "list", api=True)
         return reverse(viewname)
 
+    VERBOTEN_STRINGS = (
+        "password",
+        # https://docs.djangoproject.com/en/3.2/topics/auth/passwords/#included-hashers
+        "argon2",
+        "bcrypt",
+        "crypt",
+        "md5",
+        "pbkdf2",
+        "scrypt",
+        "sha1",
+        "sha256",
+        "sha512",
+    )
+
+    def assert_no_verboten_content(self, response):
+        """
+        Check an API response for content that should not be exposed in the API.
+
+        If a specific API has a false failure here (maybe it has security-related strings as model flags or something?),
+        its test case should overload self.VERBOTEN_STRINGS appropriately.
+        """
+        response_raw_content = response.content.decode(response.charset)
+        for verboten in self.VERBOTEN_STRINGS:
+            self.assertNotIn(verboten, response_raw_content)
+
 
 @tag("unit")
 class APIViewTestCases:
     class GetObjectViewTestCase(APITestCase):
         @override_settings(EXEMPT_VIEW_PERMISSIONS=["*"])
@@ -148,10 +173,12 @@ class APIViewTestCases:
                 self.assertIn("created", response.data)
                 self.assertIn("last_updated", response.data)
             # Fields that should be absent by default (opt-in fields):
             self.assertNotIn("computed_fields", response.data)
             self.assertNotIn("relationships", response.data)
+            # Content that should never be present:
+            self.assert_no_verboten_content(response)
 
             # If opt-in fields are supported on this model, make sure they can be opted into
 
             custom_fields_registry = registry.registry["model_features"]["custom_fields"]
             # computed fields and custom fields use the same registry
@@ -300,10 +327,11 @@ class APIViewTestCases:
 
             self.assertHttpStatus(response, status.HTTP_200_OK)
             self.assertIsInstance(response.data, dict)
             self.assertIn("results", response.data)
             self.assertEqual(len(response.data["results"]), self._get_queryset().count())
+            self.assert_no_verboten_content(response)
 
             for response_data in response.data["results"]:
                 for field in depth_fields:
                     self.assertIn(field, response_data)
                     if isinstance(response_data[field], list):
@@ -314,11 +342,12 @@ class APIViewTestCases:
                         if response_data[field] is not None:
                             self.assertIsInstance(response_data[field], dict)
                             url = response_data[field]["url"]
                             pk = response_data[field]["id"]
                             object_type = response_data[field]["object_type"]
-                            # The response should be a brief API object, containing an ID, object_type, and URL ending in the UUID of the relevant object
+                            # The response should be a brief API object, containing an ID, object_type, and a
+                            # URL ending in the UUID of the relevant object:
                             # http://nautobot.example.com/api/circuits/providers/<uuid>/
                             #                                                    ^^^^^^
                             self.assertTrue(is_uuid(url.split("/")[-2]))
                             self.assertTrue(is_uuid(pk))
 
@@ -338,10 +367,11 @@ class APIViewTestCases:
 
             self.assertHttpStatus(response, status.HTTP_200_OK)
             self.assertIsInstance(response.data, dict)
             self.assertIn("results", response.data)
             self.assertEqual(len(response.data["results"]), self._get_queryset().count())
+            self.assert_no_verboten_content(response)
 
             for response_data in response.data["results"]:
                 for field in depth_fields:
                     self.assertIn(field, response_data)
                     if isinstance(response_data[field], list):
@@ -390,10 +420,11 @@ class APIViewTestCases:
             response = self.client.get(self._get_list_url(), **self.header)
             self.assertHttpStatus(response, status.HTTP_200_OK)
             self.assertIsInstance(response.data, dict)
             self.assertIn("results", response.data)
             self.assertEqual(len(response.data["results"]), 2)
+            self.assert_no_verboten_content(response)
 
         @override_settings(EXEMPT_VIEW_PERMISSIONS=[])
         def test_list_objects_filtered(self):
             """
             GET a list of objects filtered by ID.
diff --git a/nautobot/docs/development/apps/migration/code-updates.md b/nautobot/docs/development/apps/migration/code-updates.md
index 63a09058d..618818b98 100644
--- a/nautobot/docs/development/apps/migration/code-updates.md
+++ b/nautobot/docs/development/apps/migration/code-updates.md
@@ -121,10 +121,13 @@ App Model Serializers for any models that could have a Generic Foreign Key or a
 
 `NestedSerializer` classes are no longer needed in Nautobot 2.0. If any `NestedSerializers` exist for your models, you should just remove their class definitions and references.
 
 After removing existing `NestedSerializers`, you can change the `fields` attribute in your serializers' `class Meta` to `__all__` and that will automatically include all the model's fields in the serializer, including related-model fields that would previously have required a reference to a `NestedSerializer`. If you want to exclude certain fields of the model, you can specify a list of fields you want to display in the `fields` attribute instead.
 
+!!! warning
+    Use caution around `fields = "__all__"` -- if your model has any fields that should _not_ be exposed in the REST API, you should avoid using `"__all__"` and instead use an explicit `fields` list to ensure that such fields are not exposed. In some cases, it may be appropriate to use `"__all__"` in combination with flags such as `write_only=True` on specific fields, but proceed with caution and examine the REST API data carefully to ensure that its contents are as expected.
+
 Include all model attributes:
 
 ```python
 class ExampleModelSerializer(NautobotModelSerializer):
     """Used for normal CRUD operations."""

Vulnerable code:             class Meta:
                model = relation_info.related_model
                if hasattr(base_serializer_class.Meta, "fields"):
                    fields = base_serializer_class.Meta.fields
                if hasattr(base_serializer_class.Meta, "exclude"):
                    exclude = base_serializer_class.Meta.exclude
                            # The response should be a brief API object, containing an ID, object_type, and URL ending in the UUID of the relevant object
Fixed code: Fixed incorrect inheritance of `Meta` attributes into nested serializers (`depth >= 1`).
Added check in REST API generic test cases to detect strings like `password` and `sha256` that shouldn't generally appear in REST API responses.
Fixed potential exposure of hashed user password data on certain REST API endpoints when using the `?depth=1` query parameter. For more details, please refer to [GHSA-r2hw-74xv-4gqp](https://github.com/nautobot/nautobot/security/advisories/GHSA-r2hw-74xv-4gqp).
            class Meta(base_serializer_class.Meta):
    VERBOTEN_STRINGS = (
        "password",
        # https://docs.djangoproject.com/en/3.2/topics/auth/passwords/#included-hashers
        "argon2",
        "bcrypt",
        "crypt",
        "md5",
        "pbkdf2",
        "scrypt",
        "sha1",
        "sha256",
        "sha512",
    )

    def assert_no_verboten_content(self, response):
        """
        Check an API response for content that should not be exposed in the API.

        If a specific API has a false failure here (maybe it has security-related strings as model flags or something?),
        its test case should overload self.VERBOTEN_STRINGS appropriately.
        """
        response_raw_content = response.content.decode(response.charset)
        for verboten in self.VERBOTEN_STRINGS:
            self.assertNotIn(verboten, response_raw_content)

            # Content that should never be present:
            self.assert_no_verboten_content(response)
            self.assert_no_verboten_content(response)
                            # The response should be a brief API object, containing an ID, object_type, and a
                            # URL ending in the UUID of the relevant object:
            self.assert_no_verboten_content(response)
            self.assert_no_verboten_content(response)
!!! warning
    Use caution around `fields = "__all__"` -- if your model has any fields that should _not_ be exposed in the REST API, you should avoid using `"__all__"` and instead use an explicit `fields` list to ensure that such fields are not exposed. In some cases, it may be appropriate to use `"__all__"` in combination with flags such as `write_only=True` on specific fields, but proceed with caution and examine the REST API data carefully to ensure that its contents are as expected.